package: ZipPackage: add additional check for entries STORED with

... data descriptor; only allow it for encrypted ODF entries, which
requires reading the manifest first.

Change-Id: If36d31a4cb93e7af78f48be3ed899ad9d9bb28f0
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171911
Tested-by: Jenkins
Reviewed-by: Michael Stahl <michael.stahl@allotropia.de>
(cherry picked from commit 32cad89592ec04ab552399095c91dd76afb3002c)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171949
Reviewed-by: Adolfo Jayme Barrientos <fitojb@ubuntu.com>

3 files changed, 41 insertions, 1 deletions

diff --git a/package/inc/ZipPackage.hxx b/package/inc/ZipPackage.hxx
index dbfbfe1bc17d..9848396dcf11 100644
--- a/package/inc/ZipPackage.hxx
+++ b/package/inc/ZipPackage.hxx
@@ -104,6 +104,7 @@ class ZipPackage final : public cppu::WeakImplHelper
 
     bool isLocalFile() const;
 
+    void checkZipEntriesWithDD();
     void parseManifest();
     void parseContentType();
     void getZipFileContents();
diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx
index bc886c13771c..0a1a511df1c5 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -1014,7 +1014,11 @@ sal_uInt64 ZipFile::readLOC(ZipEntry &rEntry)
         if ((rEntry.nFlag & 0x08) != 0)
         {
 #if 0
-            if (nLocMethod == STORED) // example: fdo68983.odt has this :(
+            // Unfortunately every encrypted ODF package entry hits this,
+            // because ODF requires deflated entry with value STORED and OOo/LO
+            // has always written compressed streams with data descriptor.
+            // So it is checked later in ZipPackage::checkZipEntriesWithDD()
+            if (nLocMethod == STORED)
             {
                 SAL_INFO("package", "LOC STORED with data descriptor: \"" << rEntry.sPath << "\"");
                 bBroken = true;
@@ -1387,6 +1391,11 @@ sal_Int32 ZipFile::readCEN()
             if (o3tl::checked_multiply<sal_Int64>(aEntry.nOffset, -1, aEntry.nOffset))
                 throw ZipException(u"Integer-overflow"_ustr);
 
+            if (aEntry.nMethod == STORED && aEntry.nCompressedSize != aEntry.nSize)
+            {
+                throw ZipException(u"entry STORED with inconsistent size"_ustr);
+            }
+
             aMemGrabber.skipBytes(nCommentLen);
 
             // unfortunately readLOC is required now to check the consistency
diff --git a/package/source/zippackage/ZipPackage.cxx b/package/source/zippackage/ZipPackage.cxx
index db4a61d8ddb4..1b129362b4a4 100644
--- a/package/source/zippackage/ZipPackage.cxx
+++ b/package/source/zippackage/ZipPackage.cxx
@@ -165,6 +165,31 @@ bool ZipPackage::isLocalFile() const
     return comphelper::isFileUrl(m_aURL);
 }
 
+// note: don't check for StorageFormats::ZIP, it breaks signing!
+void ZipPackage::checkZipEntriesWithDD()
+{
+    if (!m_bForceRecovery)
+    {
+        ZipEnumeration entries{m_pZipFile->entries()};
+        while (entries.hasMoreElements())
+        {
+            ZipEntry const& rEntry{*entries.nextElement()};
+            if ((rEntry.nFlag & 0x08) != 0 && rEntry.nMethod == STORED)
+            {
+                uno::Reference<XPropertySet> xStream;
+                getByHierarchicalName(rEntry.sPath) >>= xStream;
+                if (!xStream->getPropertyValue("WasEncrypted").get<bool>())
+                {
+                    SAL_INFO("package", "entry STORED with data descriptor but not encrypted: \"" << rEntry.sPath << "\"");
+                    throw ZipIOException(
+                        THROW_WHERE
+                        "entry STORED with data descriptor but not encrypted");
+                }
+            }
+        }
+    }
+}
+
 void ZipPackage::parseManifest()
 {
     if ( m_nFormat != embed::StorageFormats::PACKAGE )
@@ -419,6 +444,8 @@ void ZipPackage::parseManifest()
                     bManifestParsed = true;
                 }
 
+                checkZipEntriesWithDD(); // check before removing entries!
+
                 // now hide the manifest.xml file from user
                 xMetaInfFolder->removeByName( sManifest );
             }
@@ -665,7 +692,10 @@ void ZipPackage::getZipFileContents()
     if ( m_nFormat == embed::StorageFormats::PACKAGE )
         parseManifest();
     else if ( m_nFormat == embed::StorageFormats::OFOPXML )
+    {
         parseContentType();
+        checkZipEntriesWithDD();
+    }
 }
 
 void SAL_CALL ZipPackage::initialize( const uno::Sequence< Any >& aArguments )