package/inc/ZipPackage.hxx | 1 +
package/source/zipapi/ZipFile.cxx | 6 ++++++
package/source/zippackage/ZipPackage.cxx | 30 ++++++++++++++++++++++++++++++
3 files changed, 37 insertions(+)
New commits:
commit 32cad89592ec04ab552399095c91dd76afb3002c
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Thu Aug 15 15:49:22 2024 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Fri Aug 16 10:29:58 2024 +0200
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.st...@allotropia.de>
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 )
