From 32cad89592ec04ab552399095c91dd76afb3002c Mon Sep 17 00:00:00 2001
From: Michael Stahl <michael.stahl@allotropia.de>
Date: Thu, 15 Aug 2024 15:49:22 +0200
Subject: [PATCH] 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>
---
 package/inc/ZipPackage.hxx               |  1 +
 package/source/zipapi/ZipFile.cxx        |  6 +++++
 package/source/zippackage/ZipPackage.cxx | 30 ++++++++++++++++++++++++
 3 files changed, 37 insertions(+)

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 5c5d29435a77..fa58404ab431 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -1018,6 +1018,7 @@ sal_uInt64 ZipFile::readLOC(ZipEntry &rEntry)
             // 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 << "\"");
@@ -1389,6 +1390,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 )