comphelper/source/misc/storagehelper.cxx | 13 package/inc/ByteGrabber.hxx | 1 package/inc/ZipFile.hxx | 26 - package/inc/ZipPackage.hxx | 1 package/qa/cppunit/data/fail/ofz56826-1.zip |binary package/source/zipapi/ByteGrabber.cxx | 18 package/source/zipapi/MemoryByteGrabber.hxx | 34 + package/source/zipapi/ZipFile.cxx | 556 +++++++++++++++++++++++++--- package/source/zippackage/ZipPackage.cxx | 44 ++ package/source/zippackage/zipfileaccess.cxx | 2 sc/qa/unit/filters-test.cxx | 25 + sfx2/source/doc/objmisc.cxx | 6 sfx2/source/doc/objserv.cxx | 10 sfx2/source/doc/objstor.cxx | 2 vcl/qa/cppunit/pdfexport/pdfexport.cxx | 4 15 files changed, 666 insertions(+), 76 deletions(-)
New commits: commit 473c912734f61b809af20cb8a1d0aafa61cbc5c0 Author: Michael Stahl <[email protected]> AuthorDate: Tue Aug 13 17:59:18 2024 +0200 Commit: Michael Stahl <[email protected]> CommitDate: Mon Aug 19 20:16:20 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 <[email protected]> (cherry picked from commit 32cad89592ec04ab552399095c91dd76afb3002c) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171779 Reviewed-by: Adolfo Jayme Barrientos <[email protected]> (cherry picked from commit 525fe1a2e58a4d6164aa104e893d4f07a5709b27) diff --git a/package/inc/ZipPackage.hxx b/package/inc/ZipPackage.hxx index d1f956068327..7425eefab62d 100644 --- a/package/inc/ZipPackage.hxx +++ b/package/inc/ZipPackage.hxx @@ -103,6 +103,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 c704b0405229..77998bc09a7d 100644 --- a/package/source/zipapi/ZipFile.cxx +++ b/package/source/zipapi/ZipFile.cxx @@ -856,7 +856,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; @@ -1243,6 +1247,11 @@ sal_Int32 ZipFile::readCEN() } } + if (aEntry.nMethod == STORED && aEntry.nCompressedSize != aEntry.nSize) + { + throw ZipException("entry STORED with inconsistent size"); + } + 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 b3d43801daca..0beb7968535e 100644 --- a/package/source/zippackage/ZipPackage.cxx +++ b/package/source/zippackage/ZipPackage.cxx @@ -163,6 +163,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) + { + std::unique_ptr<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 ) @@ -360,6 +385,8 @@ void ZipPackage::parseManifest() bManifestParsed = true; } + checkZipEntriesWithDD(); // check before removing entries! + // now hide the manifest.xml file from user xMetaInfFolder->removeByName( sManifest ); } @@ -594,7 +621,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 ) commit a393ccda5fe01ab36e7b143e470c1e09b7d361d8 Author: Michael Stahl <[email protected]> AuthorDate: Fri Jul 19 20:43:05 2024 +0200 Commit: Michael Stahl <[email protected]> CommitDate: Mon Aug 19 20:16:20 2024 +0200 package: ZipFile: check Zip64 end of central directory for consistency Change-Id: Iae873ec8175922e210398ef8e0f83e148a795c2c Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170783 Reviewed-by: Michael Stahl <[email protected]> Tested-by: Jenkins (cherry picked from commit ca21cc985d57fffe7c834159b17c095206304994) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171209 Reviewed-by: Adolfo Jayme Barrientos <[email protected]> (cherry picked from commit 1d37c37ab6025bf5b107cc0c18eae5739ff02149) diff --git a/package/inc/ZipFile.hxx b/package/inc/ZipFile.hxx index e036858218d9..c65f42a637b0 100644 --- a/package/inc/ZipFile.hxx +++ b/package/inc/ZipFile.hxx @@ -94,7 +94,7 @@ private: sal_uInt64 readLOC(ZipEntry &rEntry); sal_Int32 readCEN(); - sal_Int32 findEND(); + std::tuple<sal_Int64, sal_Int64, sal_Int64> findCentralDirectory(); void recover(); static bool readExtraFields(MemoryByteGrabber& aMemGrabber, sal_Int16 nExtraLen, sal_uInt64/*&*/ nSize, sal_uInt64/*&*/ nCompressedSize, diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx index 2508de1a85ba..c704b0405229 100644 --- a/package/source/zipapi/ZipFile.cxx +++ b/package/source/zipapi/ZipFile.cxx @@ -952,18 +952,19 @@ sal_uInt64 ZipFile::readLOC(ZipEntry &rEntry) return nEnd; } -sal_Int32 ZipFile::findEND() +std::tuple<sal_Int64, sal_Int64, sal_Int64> ZipFile::findCentralDirectory() { // this method is called in constructor only, no need for mutex - sal_Int32 nLength, nPos, nEnd; Sequence < sal_Int8 > aBuffer; try { - nLength = static_cast <sal_Int32 > (aGrabber.getLength()); + sal_Int64 const nLength = aGrabber.getLength(); if (nLength < ENDHDR) - return -1; - nPos = nLength - ENDHDR - ZIP_MAXNAMELEN; - nEnd = nPos >= 0 ? nPos : 0 ; + { + throw ZipException("Zip too small!"); + } + sal_Int64 nPos = nLength - ENDHDR - ZIP_MAXNAMELEN; + sal_Int64 nEnd = nPos >= 0 ? nPos : 0; aGrabber.seek( nEnd ); @@ -973,13 +974,145 @@ sal_Int32 ZipFile::findEND() const sal_Int8 *pBuffer = aBuffer.getConstArray(); + sal_Int64 nEndPos = {}; nPos = nSize - ENDHDR; while ( nPos >= 0 ) { if (pBuffer[nPos] == 'P' && pBuffer[nPos+1] == 'K' && pBuffer[nPos+2] == 5 && pBuffer[nPos+3] == 6 ) - return nPos + nEnd; + { + nEndPos = nPos + nEnd; + break; + } nPos--; + if (nPos == 0) + { + throw ZipException("Zip END signature not found!"); + } + } + + aGrabber.seek(nEndPos + 4); + sal_uInt16 const nEndDisk = aGrabber.ReadUInt16(); + if (nEndDisk != 0) + { // only single disk is supported! + throw ZipException("invalid end (disk)" ); + } + sal_uInt16 const nEndDirDisk = aGrabber.ReadUInt16(); + if (nEndDirDisk != 0) + { + throw ZipException("invalid end (directory disk)" ); + } + sal_uInt16 const nEndDiskEntries = aGrabber.ReadUInt16(); + sal_uInt16 const nEndEntries = aGrabber.ReadUInt16(); + if (nEndDiskEntries != nEndEntries) + { + throw ZipException("invalid end (entries)" ); + } + sal_Int32 const nEndDirSize = aGrabber.ReadInt32(); + sal_Int32 const nEndDirOffset = aGrabber.ReadInt32(); + + // Zip64 end of central directory locator must immediately precede + // end of central directory record + if (20 <= nEndPos) + { + aGrabber.seek(nEndPos - 20); + Sequence<sal_Int8> aZip64EndLocator; + aGrabber.readBytes(aZip64EndLocator, 20); + MemoryByteGrabber loc64Grabber(aZip64EndLocator); + if (loc64Grabber.ReadUInt8() == 'P' + && loc64Grabber.ReadUInt8() == 'K' + && loc64Grabber.ReadUInt8() == 6 + && loc64Grabber.ReadUInt8() == 7) + { + sal_uInt32 const nLoc64Disk = loc64Grabber.ReadUInt32(); + if (nLoc64Disk != 0) + { + throw ZipException("invalid Zip64 end locator (disk)"); + } + sal_Int64 const nLoc64End64Offset = loc64Grabber.ReadUInt64(); + if (nEndPos < 20 + 56 || (nEndPos - 20 - 56) < nLoc64End64Offset + || nLoc64End64Offset < 0) + { + throw ZipException("invalid Zip64 end locator (offset)"); + } + sal_uInt32 const nLoc64Disks = loc64Grabber.ReadUInt32(); + if (nLoc64Disks != 1) + { + throw ZipException("invalid Zip64 end locator (number of disks)"); + } + aGrabber.seek(nLoc64End64Offset); + Sequence<sal_Int8> aZip64EndDirectory; + aGrabber.readBytes(aZip64EndDirectory, nEndPos - 20 - nLoc64End64Offset); + MemoryByteGrabber end64Grabber(aZip64EndDirectory); + if (end64Grabber.ReadUInt8() != 'P' + || end64Grabber.ReadUInt8() != 'K' + || end64Grabber.ReadUInt8() != 6 + || end64Grabber.ReadUInt8() != 6) + { + throw ZipException("invalid Zip64 end (signature)"); + } + sal_Int64 const nEnd64Size = end64Grabber.ReadUInt64(); + if (nEnd64Size != nEndPos - 20 - nLoc64End64Offset - 12) + { + throw ZipException("invalid Zip64 end (size)"); + } + end64Grabber.ReadUInt16(); // ignore version made by + end64Grabber.ReadUInt16(); // ignore version needed to extract + sal_uInt32 const nEnd64Disk = end64Grabber.ReadUInt32(); + if (nEnd64Disk != 0) + { + throw ZipException("invalid Zip64 end (disk)"); + } + sal_uInt32 const nEnd64EndDisk = end64Grabber.ReadUInt32(); + if (nEnd64EndDisk != 0) + { + throw ZipException("invalid Zip64 end (directory disk)"); + } + sal_uInt64 const nEnd64DiskEntries = end64Grabber.ReadUInt64(); + sal_uInt64 const nEnd64Entries = end64Grabber.ReadUInt64(); + if (nEnd64DiskEntries != nEnd64Entries) + { + throw ZipException("invalid Zip64 end (entries)"); + } + sal_Int64 const nEnd64DirSize = end64Grabber.ReadUInt64(); + sal_Int64 const nEnd64DirOffset = end64Grabber.ReadUInt64(); + if (nEndEntries != sal_uInt16(-1) && nEnd64Entries != nEndEntries) + { + throw ZipException("inconsistent Zip/Zip64 end (entries)"); + } + if (o3tl::make_unsigned(nEndDirSize) != sal_uInt32(-1) + && nEnd64DirSize != nEndDirSize) + { + throw ZipException("inconsistent Zip/Zip64 end (size)"); + } + if (o3tl::make_unsigned(nEndDirOffset) != sal_uInt32(-1) + && nEnd64DirOffset != nEndDirOffset) + { + throw ZipException("inconsistent Zip/Zip64 end (offset)"); + } + + sal_Int64 end; + if (o3tl::checked_add<sal_Int64>(nEnd64DirOffset, nEnd64DirSize, end) + || nLoc64End64Offset < end + || nEnd64DirOffset < 0 + || nLoc64End64Offset - nEnd64DirSize != nEnd64DirOffset) + { + throw ZipException("Invalid Zip64 end (bad central directory size)"); + } + + return { nEnd64Entries, nEnd64DirSize, nEnd64DirOffset }; + } } + + sal_Int32 end; + if (o3tl::checked_add<sal_Int32>(nEndDirOffset, nEndDirSize, end) + || nEndPos < end + || nEndDirOffset < 0 + || nEndPos - nEndDirSize != nEndDirOffset) + { + throw ZipException("Invalid END header (bad central directory size)"); + } + + return { nEndEntries, nEndDirSize, nEndDirOffset }; } catch ( IllegalArgumentException& ) { @@ -993,41 +1126,30 @@ sal_Int32 ZipFile::findEND() { throw ZipException("Zip END signature not found!" ); } - throw ZipException("Zip END signature not found!" ); } sal_Int32 ZipFile::readCEN() { // this method is called in constructor only, no need for mutex - sal_Int32 nCenPos = -1, nEndPos, nLocPos; - sal_uInt16 nCount; + sal_Int32 nCenPos = -1; try { - nEndPos = findEND(); - if (nEndPos == -1) - return -1; - aGrabber.seek(nEndPos + ENDTOT); - sal_uInt16 nTotal = aGrabber.ReadUInt16(); - sal_Int32 nCenLen = aGrabber.ReadInt32(); - sal_Int32 nCenOff = aGrabber.ReadInt32(); - - if ( nTotal * CENHDR > nCenLen ) - throw ZipException("invalid END header (bad entry count)" ); + auto [nTotal, nCenLen, nCenOff] = findCentralDirectory(); + nCenPos = nCenOff; // data before start of zip is not supported if ( nTotal > ZIP_MAXENTRIES ) throw ZipException("too many entries in ZIP File" ); - if ( nCenLen < 0 || nCenLen > nEndPos ) - throw ZipException("Invalid END header (bad central directory size)" ); - - nCenPos = nEndPos - nCenLen; + if (nCenLen < nTotal * CENHDR) // prevent overflow with ZIP_MAXENTRIES + throw ZipException("invalid END header (bad entry count)" ); - if ( nCenOff < 0 || nCenOff > nCenPos ) - throw ZipException("Invalid END header (bad central directory size)" ); + if (SAL_MAX_INT32 < nCenLen) + { + throw ZipException("central directory too big"); + } - nLocPos = nCenPos - nCenOff; - aGrabber.seek( nCenPos ); + aGrabber.seek(nCenPos); Sequence < sal_Int8 > aCENBuffer ( nCenLen ); sal_Int64 nRead = aGrabber.readBytes ( aCENBuffer, nCenLen ); if ( static_cast < sal_Int64 > ( nCenLen ) != nRead ) @@ -1039,6 +1161,7 @@ sal_Int32 ZipFile::readCEN() sal_Int16 nCommentLen; ::std::vector<std::pair<sal_uInt64, sal_uInt64>> unallocated = { { 0, nCenPos } }; + sal_Int64 nCount; for (nCount = 0 ; nCount < nTotal; nCount++) { sal_Int32 nTestSig = aMemGrabber.ReadInt32(); @@ -1078,8 +1201,6 @@ sal_Int32 ZipFile::readCEN() aEntry.nSize = nSize; aEntry.nOffset = nOffset; - if (o3tl::checked_add<sal_Int64>(aEntry.nOffset, nLocPos, aEntry.nOffset)) - throw ZipException("Integer-overflow"); if (o3tl::checked_multiply<sal_Int64>(aEntry.nOffset, -1, aEntry.nOffset)) throw ZipException("Integer-overflow"); commit 6f73c44b63d690f4ea945dd0646dd23b5e99b6c8 Author: Michael Stahl <[email protected]> AuthorDate: Tue Jul 16 12:12:09 2024 +0200 Commit: Michael Stahl <[email protected]> CommitDate: Mon Aug 19 20:16:20 2024 +0200 package: add additional consistency checks for local file header Check it contains same as central directory header, also check data descriptor if available. Also check there are no gaps or overlaps. This causes 3 fuzzer generated test documents to fail to load; adapt tests. Change-Id: If5813652f3bd03e90fdf95eb97e1e1523455b2b8 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170571 Tested-by: Jenkins Reviewed-by: Michael Stahl <[email protected]> (cherry picked from commit efae4fc42d5fe3c0a69757226f38efc10d101194) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170588 Reviewed-by: Thorsten Behrens <[email protected]> (cherry picked from commit cffa22ee046fd62b38692ca25dea3a6827889d9a) Disallowing Zip64 caused tdf128550.pptx be reported as corrupted, which contains a pointless Zip64 will 0 size and compressed size, so try to ignore that. Pages_5.pages contains a pointless Zip64 with the same values that are also in the local header, ignore that too... Change-Id: I750c1ea56c1ea444b9ae29b627721f4f619f0824 diff --git a/package/inc/ByteGrabber.hxx b/package/inc/ByteGrabber.hxx index 7a8e0c379596..7ee728fd0882 100644 --- a/package/inc/ByteGrabber.hxx +++ b/package/inc/ByteGrabber.hxx @@ -66,6 +66,7 @@ public: sal_uInt16 ReadUInt16(); sal_uInt32 ReadUInt32(); + sal_uInt64 ReadUInt64(); sal_Int16 ReadInt16() { return static_cast<sal_Int16>(ReadUInt16()); diff --git a/package/inc/ZipFile.hxx b/package/inc/ZipFile.hxx index 98d039fc23ef..e036858218d9 100644 --- a/package/inc/ZipFile.hxx +++ b/package/inc/ZipFile.hxx @@ -33,6 +33,7 @@ #include "EncryptionData.hxx" #include <memory> +#include <optional> #include <unordered_set> class MemoryByteGrabber; @@ -91,13 +92,13 @@ private: void getSizeAndCRC( sal_Int64 nOffset, sal_Int64 nCompressedSize, sal_Int64 *nSize, sal_Int32 *nCRC ); - void readLOC( ZipEntry &rEntry ); + sal_uInt64 readLOC(ZipEntry &rEntry); sal_Int32 readCEN(); sal_Int32 findEND(); void recover(); - static void readExtraFields(MemoryByteGrabber& aMemGrabber, sal_Int16 nExtraLen, - /*sal_uInt64& nSize, sal_uInt64& nCompressedSize, - sal_uInt64* nOffset,*/ + static bool readExtraFields(MemoryByteGrabber& aMemGrabber, sal_Int16 nExtraLen, + sal_uInt64/*&*/ nSize, sal_uInt64/*&*/ nCompressedSize, + /*::std::optional<sal_uInt64> & roOffset,*/ OUString const* pCENFilenameToCheck); public: diff --git a/package/source/zipapi/ByteGrabber.cxx b/package/source/zipapi/ByteGrabber.cxx index 29195f849eb5..29e54155d9f2 100644 --- a/package/source/zipapi/ByteGrabber.cxx +++ b/package/source/zipapi/ByteGrabber.cxx @@ -120,4 +120,22 @@ sal_uInt32 ByteGrabber::ReadUInt32() | ( pSequence[3] & 0xFF ) << 24 ); } +sal_uInt64 ByteGrabber::ReadUInt64() +{ + ::osl::MutexGuard aGuard( m_aMutex ); + + if (xStream->readBytes(aSequence, 8) != 8) + return 0; + + pSequence = aSequence.getConstArray(); + return static_cast<sal_uInt64>(pSequence[0] & 0xFF) + | static_cast<sal_uInt64>(pSequence[1] & 0xFF) << 8 + | static_cast<sal_uInt64>(pSequence[2] & 0xFF) << 16 + | static_cast<sal_uInt64>(pSequence[3] & 0xFF) << 24 + | static_cast<sal_uInt64>(pSequence[4] & 0xFF) << 32 + | static_cast<sal_uInt64>(pSequence[5] & 0xFF) << 40 + | static_cast<sal_uInt64>(pSequence[6] & 0xFF) << 48 + | static_cast<sal_uInt64>(pSequence[7] & 0xFF) << 56; +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/package/source/zipapi/MemoryByteGrabber.hxx b/package/source/zipapi/MemoryByteGrabber.hxx index 9f251a538d4c..a564dbbf5d65 100644 --- a/package/source/zipapi/MemoryByteGrabber.hxx +++ b/package/source/zipapi/MemoryByteGrabber.hxx @@ -101,6 +101,22 @@ public: nInt32 |= ( mpBuffer [mnCurrent++] & 0xFF ) << 24; return nInt32; } + + sal_uInt64 ReadUInt64() + { + if (mnCurrent + 8 > mnEnd) + return 0; + + sal_uInt64 nInt64 = mpBuffer[mnCurrent++] & 0xFF; + nInt64 |= static_cast<sal_Int64>(mpBuffer[mnCurrent++] & 0xFF) << 8; + nInt64 |= static_cast<sal_Int64>(mpBuffer[mnCurrent++] & 0xFF) << 16; + nInt64 |= static_cast<sal_Int64>(mpBuffer[mnCurrent++] & 0xFF) << 24; + nInt64 |= static_cast<sal_Int64>(mpBuffer[mnCurrent++] & 0xFF) << 32; + nInt64 |= static_cast<sal_Int64>(mpBuffer[mnCurrent++] & 0xFF) << 40; + nInt64 |= static_cast<sal_Int64>(mpBuffer[mnCurrent++] & 0xFF) << 48; + nInt64 |= static_cast<sal_Int64>(mpBuffer[mnCurrent++] & 0xFF) << 56; + return nInt64; + } }; #endif diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx index 914fe95182ed..2508de1a85ba 100644 --- a/package/source/zipapi/ZipFile.cxx +++ b/package/source/zipapi/ZipFile.cxx @@ -746,7 +746,7 @@ uno::Reference< XInputStream > ZipFile::getWrappedRawStream( return createStreamForZipEntry ( aMutexHolder, rEntry, rData, UNBUFF_STREAM_WRAPPEDRAW, true, true, aMediaType ); } -void ZipFile::readLOC( ZipEntry &rEntry ) +sal_uInt64 ZipFile::readLOC(ZipEntry &rEntry) { ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() ); @@ -762,19 +762,23 @@ void ZipFile::readLOC( ZipEntry &rEntry ) // Just verify the path and calculate the data offset and otherwise // rely on the central directory info. - aGrabber.ReadInt16(); //version - aGrabber.ReadInt16(); //flag - aGrabber.ReadInt16(); //how + aGrabber.ReadInt16(); // version - ignore any mismatch (Maven created JARs) + sal_uInt16 const nLocFlag = aGrabber.ReadUInt16(); // general purpose bit flag + sal_uInt16 const nLocMethod = aGrabber.ReadUInt16(); // compression method + // Do *not* compare timestamps, since MSO 2010 can produce documents + // with timestamp difference in the central directory entry and local + // file header. aGrabber.ReadInt32(); //time - aGrabber.ReadInt32(); //crc - aGrabber.ReadInt32(); //compressed size - aGrabber.ReadInt32(); //size + sal_uInt32 nLocCrc = aGrabber.ReadUInt32(); //crc + sal_uInt64 nLocCompressedSize = aGrabber.ReadUInt32(); //compressed size + sal_uInt64 nLocSize = aGrabber.ReadUInt32(); //size sal_Int16 nPathLen = aGrabber.ReadInt16(); sal_Int16 nExtraLen = aGrabber.ReadInt16(); rEntry.nOffset = aGrabber.getPosition() + nPathLen + nExtraLen; // FIXME64: need to read 64bit LOC + sal_Int64 nEnd = {}; // avoid -Werror=maybe-uninitialized bool bBroken = false; try @@ -802,8 +806,140 @@ void ZipFile::readLOC( ZipEntry &rEntry ) rEntry.sPath = sLOCPath; } - bBroken = rEntry.nPathLen != nPathLen - || rEntry.sPath != sLOCPath; + if (rEntry.nPathLen != nPathLen || rEntry.sPath != sLOCPath) + { + SAL_INFO("package", "LOC inconsistent name: \"" << rEntry.sPath << "\""); + bBroken = true; + } + + bool isZip64{false}; + //::std::optional<sal_uInt64> oOffset64; + if (nExtraLen != 0) + { + Sequence<sal_Int8> aExtraBuffer; + aGrabber.readBytes(aExtraBuffer, nExtraLen); + MemoryByteGrabber extraMemGrabber(aExtraBuffer); + + isZip64 = readExtraFields(extraMemGrabber, nExtraLen, + nLocSize, nLocCompressedSize, /*oOffset64,*/ &sLOCPath); + if (isZip64) + { + SAL_INFO("package", "Zip64 not supported: \"" << rEntry.sPath << "\""); + bBroken = true; // this version does NOT support Zip64 files + } + } + + // Just plain ignore bits 1 & 2 of the flag field - they are either + // purely informative, or even fully undefined (depending on method). + // Also ignore bit 11 ("Language encoding flag"): tdf125300.docx is + // example with mismatch - and the actual file names are compared in + // any case and required to be UTF-8. + if ((rEntry.nFlag & ~0x806U) != (nLocFlag & ~0x806U)) + { + SAL_INFO("package", "LOC inconsistent flag: \"" << rEntry.sPath << "\""); + bBroken = true; + } + + // TODO: "older versions with encrypted streams write mismatching DEFLATE/STORE" ??? + if (rEntry.nMethod != nLocMethod) + { + SAL_INFO("package", "LOC inconsistent method: \"" << rEntry.sPath << "\""); + bBroken = true; + } + + if (o3tl::checked_add<sal_Int64>(rEntry.nOffset, rEntry.nCompressedSize, nEnd)) + { + throw ZipException("Integer-overflow"); + } + + // read "data descriptor" - this can be 12, 16, 20, or 24 bytes in size + if ((rEntry.nFlag & 0x08) != 0) + { +#if 0 + if (nLocMethod == STORED) // example: fdo68983.odt has this :( + { + SAL_INFO("package", "LOC STORED with data descriptor: \"" << rEntry.sPath << "\""); + bBroken = true; + } + else +#endif + { + decltype(nLocCrc) nDDCrc; + decltype(nLocCompressedSize) nDDCompressedSize; + decltype(nLocSize) nDDSize; + aGrabber.seek(aGrabber.getPosition() + rEntry.nCompressedSize); + sal_uInt32 nTemp = aGrabber.ReadUInt32(); + if (nTemp == 0x08074b50) // APPNOTE says PK78 is optional??? + { + nDDCrc = aGrabber.ReadUInt32(); + } + else + { + nDDCrc = nTemp; + } + if (isZip64) + { + nDDCompressedSize = aGrabber.ReadUInt64(); + nDDSize = aGrabber.ReadUInt64(); + } + else + { + nDDCompressedSize = aGrabber.ReadUInt32(); + nDDSize = aGrabber.ReadUInt32(); + } + if (nEnd < aGrabber.getPosition()) + { + nEnd = aGrabber.getPosition(); + } + else + { + SAL_INFO("package", "LOC invalid size: \"" << rEntry.sPath << "\""); + bBroken = true; + } + // tdf91429.docx has same values in LOC and in (superfluous) DD + if ((nLocCrc == 0 || nLocCrc == nDDCrc) + && (nLocCompressedSize == 0 || nLocCompressedSize == sal_uInt64(-1) || nLocCompressedSize == nDDCompressedSize) + && (nLocSize == 0 || nLocSize == sal_uInt64(-1) || nLocSize == nDDSize)) + + { + nLocCrc = nDDCrc; + nLocCompressedSize = nDDCompressedSize; + nLocSize = nDDSize; + } + else + { + SAL_INFO("package", "LOC non-0 with data descriptor: \"" << rEntry.sPath << "\""); + bBroken = true; + } + } + } + + // unit test file export64.zip has nLocCrc/nLocCS/nLocSize = 0 on mimetype + if (nLocCrc != 0 && static_cast<sal_uInt32>(rEntry.nCrc) != nLocCrc) + { + SAL_INFO("package", "LOC inconsistent CRC: \"" << rEntry.sPath << "\""); + bBroken = true; + } + + if (nLocCompressedSize != 0 && static_cast<sal_uInt64>(rEntry.nCompressedSize) != nLocCompressedSize) + { + SAL_INFO("package", "LOC inconsistent compressed size: \"" << rEntry.sPath << "\""); + bBroken = true; + } + + if (nLocSize != 0 && static_cast<sal_uInt64>(rEntry.nSize) != nLocSize) + { + SAL_INFO("package", "LOC inconsistent size: \"" << rEntry.sPath << "\""); + bBroken = true; + } + +#if 0 + if (oOffset64 && o3tl::make_unsigned(nPos) != *oOffset64) + { + SAL_INFO("package", "LOC inconsistent offset: \"" << rEntry.sPath << "\""); + bBroken = true; + } +#endif } catch(...) { @@ -812,6 +948,8 @@ void ZipFile::readLOC( ZipEntry &rEntry ) if ( bBroken && !bRecoveryMode ) throw ZipIOException("The stream seems to be broken!" ); + + return nEnd; } sal_Int32 ZipFile::findEND() @@ -899,7 +1037,7 @@ sal_Int32 ZipFile::readCEN() ZipEntry aEntry; sal_Int16 nCommentLen; - sal_Int64 nMinOffset{nEndPos}; + ::std::vector<std::pair<sal_uInt64, sal_uInt64>> unallocated = { { 0, nCenPos } }; for (nCount = 0 ; nCount < nTotal; nCount++) { @@ -942,7 +1080,6 @@ sal_Int32 ZipFile::readCEN() if (o3tl::checked_add<sal_Int64>(aEntry.nOffset, nLocPos, aEntry.nOffset)) throw ZipException("Integer-overflow"); - nMinOffset = std::min(nMinOffset, aEntry.nOffset); if (o3tl::checked_multiply<sal_Int64>(aEntry.nOffset, -1, aEntry.nOffset)) throw ZipException("Integer-overflow"); @@ -970,11 +1107,76 @@ sal_Int32 ZipFile::readCEN() if (aEntry.nExtraLen>0) { - readExtraFields(aMemGrabber, aEntry.nExtraLen, /*nSize, nCompressedSize, &nOffset,*/ &aEntry.sPath); + //::std::optional<sal_uInt64> oOffset64; + bool const isZip64 = readExtraFields(aMemGrabber, aEntry.nExtraLen, nSize, nCompressedSize, /*oOffset64,*/ &aEntry.sPath); +#if 0 + if (oOffset64) + { + nOffset = *oOffset64; + } +#endif + if (isZip64) + { + SAL_INFO("package", "Zip64 not supported: \"" << aEntry.sPath << "\""); + throw ZipException("Zip64 not supported"); + } } aMemGrabber.skipBytes(nCommentLen); + // unfortunately readLOC is required now to check the consistency + assert(aEntry.nOffset <= 0); + sal_uInt64 const nStart{ o3tl::make_unsigned(-aEntry.nOffset) }; + sal_uInt64 const nEnd = readLOC(aEntry); + assert(nStart < nEnd); + for (auto it = unallocated.begin(); ; ++it) + { + if (it == unallocated.end()) + { + throw ZipException("overlapping entries"); + } + if (nStart < it->first) + { + throw ZipException("overlapping entries"); + } + else if (it->first == nStart) + { + if (it->second == nEnd) + { + unallocated.erase(it); + break; + } + else if (nEnd < it->second) + { + it->first = nEnd; + break; + } + else + { + throw ZipException("overlapping entries"); + } + } + else if (nStart < it->second) + { + if (nEnd < it->second) + { + auto const temp{it->first}; + it->first = nEnd; + unallocated.insert(it, { temp, nStart }); + break; + } + else if (nEnd == it->second) + { + it->second = nStart; + break; + } + else + { + throw ZipException("overlapping entries"); + } + } + } + if (aEntries.find(aEntry.sPath) != aEntries.end()) { SAL_INFO("package", "Duplicate CEN entry: \"" << aEntry.sPath << "\""); @@ -992,9 +1194,9 @@ sal_Int32 ZipFile::readCEN() if (nCount != nTotal) throw ZipException("Count != Total" ); - if (nMinOffset != 0) + if (!unallocated.empty()) { - throw ZipException("Extra bytes at beginning of zip file"); + throw ZipException("Zip file has holes! It will leak!"); } } catch ( IllegalArgumentException & ) @@ -1005,37 +1207,48 @@ sal_Int32 ZipFile::readCEN() return nCenPos; } -void ZipFile::readExtraFields(MemoryByteGrabber& aMemGrabber, sal_Int16 nExtraLen, - /*sal_uInt64& nSize, sal_uInt64& nCompressedSize, - sal_uInt64* nOffset, */OUString const*const pCENFilenameToCheck) +bool ZipFile::readExtraFields(MemoryByteGrabber& aMemGrabber, sal_Int16 nExtraLen, + sal_uInt64/*&*/ nLocSize, sal_uInt64/*&*/ nLocCompressedSize, +// std::optional<sal_uInt64> & roOffset, + OUString const*const pCENFilenameToCheck) { + bool isZip64{false}; while (nExtraLen > 0) // Extensible data fields { sal_Int16 nheaderID = aMemGrabber.ReadInt16(); sal_uInt16 dataSize = aMemGrabber.ReadUInt16(); -#if 0 if (nheaderID == 1) // Load Zip64 Extended Information Extra Field { // Datasize should be 28byte but some files have less (maybe non standard?) - nSize = aMemGrabber.ReadUInt64(); + auto const nSize = aMemGrabber.ReadUInt64(); + if (nSize != 0 && nSize != nLocSize) + { // this may look weird but then there is tdf128550.pptx produced reportedly by an Apple product + isZip64 = true; + } sal_uInt16 nReadSize = 8; if (dataSize >= 16) { - nCompressedSize = aMemGrabber.ReadUInt64(); + auto const nCompressedSize = aMemGrabber.ReadUInt64(); + if (nCompressedSize != 0 && nCompressedSize != nLocCompressedSize) + { + isZip64 = true; + } nReadSize = 16; - if (dataSize >= 24 && nOffset) + if (dataSize >= 24 /*&& roOffset*/) { - *nOffset = aMemGrabber.ReadUInt64(); + isZip64 = true; +#if 0 + roOffset.emplace(aMemGrabber.ReadUInt64()); nReadSize = 24; // 4 byte should be "Disk Start Number" but we not need it +#endif } } if (dataSize > nReadSize) aMemGrabber.skipBytes(dataSize - nReadSize); } -#endif // Info-ZIP Unicode Path Extra Field - pointless as we expect UTF-8 in CEN already - if (nheaderID == 0x7075 && pCENFilenameToCheck) // ignore in recovery mode + else if (nheaderID == 0x7075 && pCENFilenameToCheck) // ignore in recovery mode { if (aMemGrabber.remainingSize() < dataSize) { @@ -1069,6 +1282,7 @@ void ZipFile::readExtraFields(MemoryByteGrabber& aMemGrabber, sal_Int16 nExtraLe } nExtraLen -= dataSize + 4; } + return isZip64; } void ZipFile::recover() diff --git a/sc/qa/unit/filters-test.cxx b/sc/qa/unit/filters-test.cxx index 33887536e681..7b31278409a4 100644 --- a/sc/qa/unit/filters-test.cxx +++ b/sc/qa/unit/filters-test.cxx @@ -30,6 +30,12 @@ #include <scmod.hxx> #include <svx/svdpage.hxx> +#if 0 +#include <comphelper/propertyvalue.hxx> +#include <comphelper/processfactory.hxx> + +#include <com/sun/star/frame/Desktop.hpp> +#endif using namespace ::com::sun::star; using namespace ::com::sun::star::uno; @@ -850,8 +856,23 @@ void ScFiltersTest::testSortWithFormattingXLS() // just needs to not crash on recalc void ScFiltersTest::testForcepoint107() { - ScDocShellRef xDocSh = loadDoc(u"forcepoint107.", FORMAT_XLSX, true); - xDocSh->DoHardRecalc(); + // It expectedly fails to load normally + CPPUNIT_ASSERT(!loadDoc(u"forcepoint107.", FORMAT_XLSX, true)); + + // type detection fails on this branch due to the broken zip, can't test +#if 0 + // importing it must succeed with RepairPackage set to true. + uno::Sequence<beans::PropertyValue> aParams + = { comphelper::makePropertyValue("RepairPackage", true) }; + OUString url; + createFileURL(u"forcepoint107.", u"xlsx", url); + + css::uno::Reference<css::frame::XDesktop2> xDesktop(css::frame::Desktop::create(comphelper::getProcessComponentContext())); + m_xCalcComponent = xDesktop->loadComponentFromURL(url, "_default", 0, aParams); + + ScDocShell* pDocSh = getScDocShell(); + pDocSh->DoHardRecalc(); +#endif } ScFiltersTest::ScFiltersTest() diff --git a/vcl/qa/cppunit/pdfexport/pdfexport.cxx b/vcl/qa/cppunit/pdfexport/pdfexport.cxx index 344a74f87521..5d6fa51d7778 100644 --- a/vcl/qa/cppunit/pdfexport/pdfexport.cxx +++ b/vcl/qa/cppunit/pdfexport/pdfexport.cxx @@ -1665,7 +1665,9 @@ void PdfExportTest::testTdf113143() void PdfExportTest::testForcePoint71() { // I just care it doesn't crash - topdf("forcepoint71.key"); + // numerous Zip errors are detected now and libetonyek cannot RepairPackage + OUString aURL = m_directories.getURLFromSrc(DATA_DIRECTORY) + u"forcepoint71.key"; + CPPUNIT_ASSERT_ASSERTION_FAIL(loadFromDesktop(aURL)); } void PdfExportTest::testTdf115262() commit 83d69fab9d53e6fd08d3b84236156e547caff461 Author: Michael Stahl <[email protected]> AuthorDate: Wed Jul 17 12:04:13 2024 +0200 Commit: Michael Stahl <[email protected]> CommitDate: Mon Aug 19 19:14:09 2024 +0200 package: don't check case insensitive duplicates for ZIP package Turns out there's a TexMaths extension that contains files with names differing only in case. https://ask.libreoffice.org/t/zipexception-when-installing-an-extension/108256 There isn't a separate ZipPackage mode for OXT so just don't check in the ZIP mode. Change-Id: I7680c93f5f24ac566a59b131b36d855bd85100b9 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170616 Tested-by: Jenkins Reviewed-by: Michael Stahl <[email protected]> (cherry picked from commit 0fb25ce9ff9a3ede8d43ee1502c44b4c02135b3f) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170623 Reviewed-by: Xisco Fauli <[email protected]> (cherry picked from commit bf9ece7cf6d184675fac1eba88ebcb04bf81442f) diff --git a/package/inc/ZipFile.hxx b/package/inc/ZipFile.hxx index 54de57d78645..98d039fc23ef 100644 --- a/package/inc/ZipFile.hxx +++ b/package/inc/ZipFile.hxx @@ -56,9 +56,14 @@ class ZipEnumeration; class ZipFile { +public: + enum class Checks { Default, CheckInsensitive }; + +private: rtl::Reference<comphelper::RefCountedMutex> m_aMutexHolder; std::unordered_set<OUString> m_EntriesInsensitive; + Checks m_Checks; EntryHash aEntries; ByteGrabber aGrabber; @@ -97,16 +102,12 @@ class ZipFile public: - ZipFile( const rtl::Reference<comphelper::RefCountedMutex>& aMutexHolder, - css::uno::Reference < css::io::XInputStream > const &xInput, - const css::uno::Reference < css::uno::XComponentContext > &rxContext, - bool bInitialise ); - ZipFile( const rtl::Reference<comphelper::RefCountedMutex>& aMutexHolder, css::uno::Reference < css::io::XInputStream > const &xInput, const css::uno::Reference < css::uno::XComponentContext > &rxContext, bool bInitialise, - bool bForceRecover ); + bool bForceRecover, + Checks checks); ~ZipFile(); diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx index 17a3a3f15079..914fe95182ed 100644 --- a/package/source/zipapi/ZipFile.cxx +++ b/package/source/zipapi/ZipFile.cxx @@ -76,27 +76,10 @@ using ZipUtils::Inflater; ZipFile::ZipFile( const rtl::Reference<comphelper::RefCountedMutex>& aMutexHolder, uno::Reference < XInputStream > const &xInput, const uno::Reference < XComponentContext > & rxContext, - bool bInitialise ) -: m_aMutexHolder( aMutexHolder ) -, aGrabber( xInput ) -, aInflater( true ) -, xStream(xInput) -, m_xContext ( rxContext ) -, bRecoveryMode( false ) -{ - if (bInitialise && readCEN() == -1 ) - { - aEntries.clear(); - m_EntriesInsensitive.clear(); - throw ZipException( "stream data looks to be broken" ); - } -} - -ZipFile::ZipFile( const rtl::Reference< comphelper::RefCountedMutex >& aMutexHolder, - uno::Reference < XInputStream > const &xInput, - const uno::Reference < XComponentContext > & rxContext, - bool bInitialise, bool bForceRecovery) + bool bInitialise, bool bForceRecovery, + Checks const checks) : m_aMutexHolder( aMutexHolder ) +, m_Checks(checks) , aGrabber( xInput ) , aInflater( true ) , xStream(xInput) @@ -999,7 +982,7 @@ sal_Int32 ZipFile::readCEN() } // this is required for OOXML, but not for ODF auto const lowerPath(aEntry.sPath.toAsciiLowerCase()); - if (!m_EntriesInsensitive.insert(lowerPath).second) + if (!m_EntriesInsensitive.insert(lowerPath).second && m_Checks == Checks::CheckInsensitive) { SAL_INFO("package", "Duplicate CEN entry (case insensitive): \"" << aEntry.sPath << "\""); throw ZipException("Duplicate CEN entry (case insensitive)"); diff --git a/package/source/zippackage/ZipPackage.cxx b/package/source/zippackage/ZipPackage.cxx index d8dcfcb9c2cc..b3d43801daca 100644 --- a/package/source/zippackage/ZipPackage.cxx +++ b/package/source/zippackage/ZipPackage.cxx @@ -771,7 +771,9 @@ void SAL_CALL ZipPackage::initialize( const uno::Sequence< Any >& aArguments ) OUString message; try { - m_pZipFile = std::make_unique<ZipFile>(m_aMutexHolder, m_xContentStream, m_xContext, true, m_bForceRecovery); + m_pZipFile = std::make_unique<ZipFile>(m_aMutexHolder, m_xContentStream, m_xContext, true, + m_bForceRecovery, + m_nFormat == embed::StorageFormats::ZIP ? ZipFile::Checks::Default : ZipFile::Checks::CheckInsensitive); getZipFileContents(); } catch ( IOException & e ) @@ -1147,7 +1149,9 @@ void ZipPackage::ConnectTo( const uno::Reference< io::XInputStream >& xInStream if ( m_pZipFile ) m_pZipFile->setInputStream( m_xContentStream ); else - m_pZipFile = std::make_unique<ZipFile>(m_aMutexHolder, m_xContentStream, m_xContext, false); + m_pZipFile = std::make_unique<ZipFile>(m_aMutexHolder, m_xContentStream, m_xContext, false, + false, + m_nFormat == embed::StorageFormats::ZIP ? ZipFile::Checks::Default : ZipFile::Checks::CheckInsensitive); } namespace diff --git a/package/source/zippackage/zipfileaccess.cxx b/package/source/zippackage/zipfileaccess.cxx index 31e38aff1dcd..f86a5fde9984 100644 --- a/package/source/zippackage/zipfileaccess.cxx +++ b/package/source/zippackage/zipfileaccess.cxx @@ -244,7 +244,7 @@ void SAL_CALL OZipFileAccess::initialize( const uno::Sequence< uno::Any >& aArgu m_aMutexHolder, m_xContentStream, m_xContext, - true ); + true, false, ZipFile::Checks::Default); } // XNameAccess commit ad416f52d6eb9e49d522dab9cc0370a10fc0eff2 Author: Michael Stahl <[email protected]> AuthorDate: Tue Jul 9 17:22:15 2024 +0200 Commit: Michael Stahl <[email protected]> CommitDate: Mon Aug 19 17:53:51 2024 +0200 package: ZipFile: don't accept duplicate entries (case insensitive) This is required for OOXML, but not for ODF. Unclear if there are use cases for this with ODF, can add some conditions if it turns out to be a problem. Change-Id: I3810da5c2273574135d133b4a9bbad98dc97af44 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170223 Tested-by: Jenkins Reviewed-by: Michael Stahl <[email protected]> (cherry picked from commit 4833f131243bdb409ddfaff8b4db87d4ed2af98f) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170282 Reviewed-by: Christian Lohmaier <[email protected]> (cherry picked from commit 92564a17fe68066f6e9135c6f79fb6319b192f22) diff --git a/package/inc/ZipFile.hxx b/package/inc/ZipFile.hxx index ae5c6d5aeef8..54de57d78645 100644 --- a/package/inc/ZipFile.hxx +++ b/package/inc/ZipFile.hxx @@ -33,6 +33,7 @@ #include "EncryptionData.hxx" #include <memory> +#include <unordered_set> class MemoryByteGrabber; namespace com { namespace sun { namespace star { @@ -57,6 +58,8 @@ class ZipFile { rtl::Reference<comphelper::RefCountedMutex> m_aMutexHolder; + std::unordered_set<OUString> m_EntriesInsensitive; + EntryHash aEntries; ByteGrabber aGrabber; ZipUtils::Inflater aInflater; diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx index b2c730de672d..17a3a3f15079 100644 --- a/package/source/zipapi/ZipFile.cxx +++ b/package/source/zipapi/ZipFile.cxx @@ -87,6 +87,7 @@ ZipFile::ZipFile( const rtl::Reference<comphelper::RefCountedMutex>& aMutexHolde if (bInitialise && readCEN() == -1 ) { aEntries.clear(); + m_EntriesInsensitive.clear(); throw ZipException( "stream data looks to be broken" ); } } @@ -111,6 +112,7 @@ ZipFile::ZipFile( const rtl::Reference< comphelper::RefCountedMutex >& aMutexHol else if ( readCEN() == -1 ) { aEntries.clear(); + m_EntriesInsensitive.clear(); throw ZipException("stream data looks to be broken" ); } } @@ -990,15 +992,19 @@ sal_Int32 ZipFile::readCEN() aMemGrabber.skipBytes(nCommentLen); - if (auto it = aEntries.find(aEntry.sPath); it == aEntries.end()) - { - aEntries[aEntry.sPath] = aEntry; - } - else + if (aEntries.find(aEntry.sPath) != aEntries.end()) { SAL_INFO("package", "Duplicate CEN entry: \"" << aEntry.sPath << "\""); throw ZipException("Duplicate CEN entry"); } + // this is required for OOXML, but not for ODF + auto const lowerPath(aEntry.sPath.toAsciiLowerCase()); + if (!m_EntriesInsensitive.insert(lowerPath).second) + { + SAL_INFO("package", "Duplicate CEN entry (case insensitive): \"" << aEntry.sPath << "\""); + throw ZipException("Duplicate CEN entry (case insensitive)"); + } + aEntries[aEntry.sPath] = aEntry; } if (nCount != nTotal) @@ -1172,6 +1178,13 @@ void ZipFile::recover() aEntry.nSize = 0; } + auto const lowerPath(aEntry.sPath.toAsciiLowerCase()); + if (m_EntriesInsensitive.find(lowerPath) != m_EntriesInsensitive.end()) + { // this is required for OOXML, but not for ODF + nPos += 4; + continue; + } + m_EntriesInsensitive.insert(lowerPath); aEntries.emplace( aEntry.sPath, aEntry ); } } commit d3cb69bee426865cadc62b3010b72be878449405 Author: Michael Stahl <[email protected]> AuthorDate: Fri Jul 5 13:55:31 2024 +0200 Commit: Michael Stahl <[email protected]> CommitDate: Mon Aug 19 17:51:23 2024 +0200 package: avoid throwing RuntimeException in getZipFileContents() Translate it to ZipIOException. Change-Id: I7a07a59c0ba301b92f31696355c73ccbdf119ff8 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170013 Tested-by: Jenkins Reviewed-by: Michael Stahl <[email protected]> (cherry picked from commit c409c83d777fdb6291c7cd03186b69fe4e7fd902) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170030 Reviewed-by: Xisco Fauli <[email protected]> (cherry picked from commit bcb6130ef8ab66a99347945c1c30ec0112d6940e) diff --git a/package/source/zippackage/ZipPackage.cxx b/package/source/zippackage/ZipPackage.cxx index 7b2e705e45e5..d8dcfcb9c2cc 100644 --- a/package/source/zippackage/ZipPackage.cxx +++ b/package/source/zippackage/ZipPackage.cxx @@ -555,7 +555,11 @@ void ZipPackage::getZipFileContents() if ( !pCurrent->hasByName( sTemp ) ) { ZipPackageFolder* pPkgFolder = new ZipPackageFolder(m_xContext, m_nFormat, m_bAllowRemoveOnInsert); - pPkgFolder->setName( sTemp ); + try { + pPkgFolder->setName( sTemp ); + } catch (uno::RuntimeException const& e) { + throw css::packages::zip::ZipIOException(e.Message); + } pPkgFolder->doSetParent( pCurrent ); pCurrent = pPkgFolder; } commit 11f1bd8583959eeadaf03886ee68db4811cfbb4d Author: Michael Stahl <[email protected]> AuthorDate: Thu Jul 4 17:52:49 2024 +0200 Commit: Michael Stahl <[email protected]> CommitDate: Mon Aug 19 17:48:12 2024 +0200 package: ZipFile: treat junk at the start of zip as invalid Probably the only legitimate use of such is self-extracting archives, irrelevant for LO. ofz56826-1.zip is an example; given what Info-Zip unzip prints about this file we don't want to successfully open it. Change-Id: I9568710227e4a152f9dc7bc356184394d7da8eba Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170002 Tested-by: Jenkins Reviewed-by: Michael Stahl <[email protected]> (cherry picked from commit 2afdc61dd3138b383fb73dae2242ba1a9c8de901) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170009 Reviewed-by: Xisco Fauli <[email protected]> (cherry picked from commit fca236e6fd7f1b42d8ed23907913036d8140d651) diff --git a/package/qa/cppunit/data/pass/ofz56826-1.zip b/package/qa/cppunit/data/fail/ofz56826-1.zip similarity index 100% rename from package/qa/cppunit/data/pass/ofz56826-1.zip rename to package/qa/cppunit/data/fail/ofz56826-1.zip diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx index 35565d162f86..b2c730de672d 100644 --- a/package/source/zipapi/ZipFile.cxx +++ b/package/source/zipapi/ZipFile.cxx @@ -914,6 +914,7 @@ sal_Int32 ZipFile::readCEN() ZipEntry aEntry; sal_Int16 nCommentLen; + sal_Int64 nMinOffset{nEndPos}; for (nCount = 0 ; nCount < nTotal; nCount++) { @@ -956,6 +957,7 @@ sal_Int32 ZipFile::readCEN() if (o3tl::checked_add<sal_Int64>(aEntry.nOffset, nLocPos, aEntry.nOffset)) throw ZipException("Integer-overflow"); + nMinOffset = std::min(nMinOffset, aEntry.nOffset); if (o3tl::checked_multiply<sal_Int64>(aEntry.nOffset, -1, aEntry.nOffset)) throw ZipException("Integer-overflow"); @@ -1001,6 +1003,10 @@ sal_Int32 ZipFile::readCEN() if (nCount != nTotal) throw ZipException("Count != Total" ); + if (nMinOffset != 0) + { + throw ZipException("Extra bytes at beginning of zip file"); + } } catch ( IllegalArgumentException & ) { commit c5173e02c29cdd04ef55a6d6bf29d5e80dc7686a Author: Michael Stahl <[email protected]> AuthorDate: Thu Jul 4 14:07:25 2024 +0200 Commit: Michael Stahl <[email protected]> CommitDate: Mon Aug 19 17:47:26 2024 +0200 comphelper: treat zip file path segments '.' and '..' as invalid This will prevent also opening with RepairPackage, would need to adapt ZipPackage::getZipFileContents() a bit, but let's hope nobody acutally has such files. Also treat path that starts with "/" as invalid, presumably it's not allowed by APPNOTE.TXT: "The name of the file, with optional relative path." Change-Id: Ic694ea2fb34f5de1d490a9a251cf56e4004e9673 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169994 Reviewed-by: Michael Stahl <[email protected]> Tested-by: Jenkins (cherry picked from commit 6005260078c126bf3f1cf4d6f1ebb631453f5ac7) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169964 Reviewed-by: Xisco Fauli <[email protected]> (cherry picked from commit fda9ae6f1cef7512f0560828f67553b91641bd26) diff --git a/comphelper/source/misc/storagehelper.cxx b/comphelper/source/misc/storagehelper.cxx index 4c9d55b68251..3fb7f649f477 100644 --- a/comphelper/source/misc/storagehelper.cxx +++ b/comphelper/source/misc/storagehelper.cxx @@ -568,10 +568,17 @@ bool OStorageHelper::IsValidZipEntryFileName( const OUString& aName, bool bSlash bool OStorageHelper::IsValidZipEntryFileName( const sal_Unicode *pChar, sal_Int32 nLength, bool bSlashAllowed ) { + long nDots{0}; for ( sal_Int32 i = 0; i < nLength; i++ ) { switch ( pChar[i] ) { + case '.': + if (nDots != -1) + { + ++nDots; + } + break; case '\': case '?': case '<': @@ -581,15 +588,17 @@ bool OStorageHelper::IsValidZipEntryFileName( case ':': return false; case '/': - if ( !bSlashAllowed ) + if (!bSlashAllowed || nDots == 1 || nDots == 2 || i == 0) return false; + nDots = 0; break; default: + nDots = -1; if ( pChar[i] < 32 || (pChar[i] >= 0xD800 && pChar[i] <= 0xDFFF) ) return false; } } - return true; + return nDots != 1 && nDots != 2; } commit e232736f1bfc508b17d213f87563bfc09de55b9d Author: Michael Stahl <[email protected]> AuthorDate: Tue Jul 2 11:50:09 2024 +0200 Commit: Michael Stahl <[email protected]> CommitDate: Mon Aug 19 17:44:02 2024 +0200 package: ZipFile: check Info-ZIP Unicode Path Extra Field Change-Id: I829eb449e8a0947341f066399be549f56b0f02da Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169882 Tested-by: Jenkins Reviewed-by: Michael Stahl <[email protected]> (cherry picked from commit 67dca5a47d1be1b38edee7d0d81fa9adfce5a22e) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169929 Reviewed-by: Caolán McNamara <[email protected]> (cherry picked from commit 1150584be851b311067ee6482f51da1ad3b12636) ofz#56826 Heap-use-after-free since: commit abda72eeac19b18c22f57d5443c3955a463605d7 Date: Mon Feb 20 00:32:22 2023 +0100 tdf#82984 tdf#94915 zip64 support (import + export) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/148526 Tested-by: Jenkins Reviewed-by: Caolán McNamara <[email protected]> (cherry picked from commit 59b0f676758dd752457c84fb4159f6446d74e8a4) Change-Id: I11d2247004e3fb20fe2e443f0d4aa755290aa667 diff --git a/package/inc/ZipFile.hxx b/package/inc/ZipFile.hxx index 58755d4ad2c7..ae5c6d5aeef8 100644 --- a/package/inc/ZipFile.hxx +++ b/package/inc/ZipFile.hxx @@ -34,6 +34,7 @@ #include <memory> +class MemoryByteGrabber; namespace com { namespace sun { namespace star { namespace uno { class XComponentContext; } namespace ucb { class XProgressHandler; } @@ -86,6 +87,10 @@ class ZipFile sal_Int32 readCEN(); sal_Int32 findEND(); void recover(); + static void readExtraFields(MemoryByteGrabber& aMemGrabber, sal_Int16 nExtraLen, + /*sal_uInt64& nSize, sal_uInt64& nCompressedSize, + sal_uInt64* nOffset,*/ + OUString const* pCENFilenameToCheck); public: diff --git a/package/qa/cppunit/data/pass/ofz56826-1.zip b/package/qa/cppunit/data/pass/ofz56826-1.zip new file mode 100644 index 000000000000..b9acfe34da14 Binary files /dev/null and b/package/qa/cppunit/data/pass/ofz56826-1.zip differ diff --git a/package/source/zipapi/MemoryByteGrabber.hxx b/package/source/zipapi/MemoryByteGrabber.hxx index 9f52204ead67..9f251a538d4c 100644 --- a/package/source/zipapi/MemoryByteGrabber.hxx +++ b/package/source/zipapi/MemoryByteGrabber.hxx @@ -51,6 +51,14 @@ public: mnCurrent += nBytesToSkip; } + sal_Int8 ReadUInt8() + { + if (mnCurrent + 1 > mnEnd) + return 0; + sal_uInt8 nInt8 = mpBuffer[mnCurrent++]; + return nInt8; + } + // XSeekable chained... sal_Int16 ReadInt16() { @@ -60,6 +68,16 @@ public: nInt16 |= ( mpBuffer[mnCurrent++] & 0xFF ) << 8; return nInt16; } + + sal_Int16 ReadUInt16() + { + if (mnCurrent + 2 > mnEnd ) + return 0; + sal_uInt16 nInt16 = mpBuffer[mnCurrent++] & 0xFF; + nInt16 |= ( mpBuffer[mnCurrent++] & 0xFF ) << 8; + return nInt16; + } + sal_Int32 ReadInt32() { if (mnCurrent + 4 > mnEnd ) diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx index 4c36b5b767d5..35565d162f86 100644 --- a/package/source/zipapi/ZipFile.cxx +++ b/package/source/zipapi/ZipFile.cxx @@ -31,6 +31,7 @@ #include <comphelper/storagehelper.hxx> #include <comphelper/processfactory.hxx> #include <rtl/digest.h> +#include <rtl/crc.h> #include <sal/log.hxx> #include <o3tl/safeint.hxx> #include <osl/diagnose.h> @@ -978,7 +979,14 @@ sal_Int32 ZipFile::readCEN() if ( !::comphelper::OStorageHelper::IsValidZipEntryFileName( aEntry.sPath, true ) ) throw ZipException("Zip entry has an invalid name." ); - aMemGrabber.skipBytes( aEntry.nPathLen + aEntry.nExtraLen + nCommentLen ); + aMemGrabber.skipBytes(aEntry.nPathLen); + + if (aEntry.nExtraLen>0) + { + readExtraFields(aMemGrabber, aEntry.nExtraLen, /*nSize, nCompressedSize, &nOffset,*/ &aEntry.sPath); + } + + aMemGrabber.skipBytes(nCommentLen); if (auto it = aEntries.find(aEntry.sPath); it == aEntries.end()) { @@ -1002,6 +1010,72 @@ sal_Int32 ZipFile::readCEN() return nCenPos; } +void ZipFile::readExtraFields(MemoryByteGrabber& aMemGrabber, sal_Int16 nExtraLen, + /*sal_uInt64& nSize, sal_uInt64& nCompressedSize, + sal_uInt64* nOffset, */OUString const*const pCENFilenameToCheck) +{ + while (nExtraLen > 0) // Extensible data fields + { + sal_Int16 nheaderID = aMemGrabber.ReadInt16(); + sal_uInt16 dataSize = aMemGrabber.ReadUInt16(); +#if 0 + if (nheaderID == 1) // Load Zip64 Extended Information Extra Field + { + // Datasize should be 28byte but some files have less (maybe non standard?) + nSize = aMemGrabber.ReadUInt64(); + sal_uInt16 nReadSize = 8; + if (dataSize >= 16) + { + nCompressedSize = aMemGrabber.ReadUInt64(); + nReadSize = 16; + if (dataSize >= 24 && nOffset) + { + *nOffset = aMemGrabber.ReadUInt64(); + nReadSize = 24; + // 4 byte should be "Disk Start Number" but we not need it + } + } + if (dataSize > nReadSize) + aMemGrabber.skipBytes(dataSize - nReadSize); + } +#endif + // Info-ZIP Unicode Path Extra Field - pointless as we expect UTF-8 in CEN already + if (nheaderID == 0x7075 && pCENFilenameToCheck) // ignore in recovery mode + { + if (aMemGrabber.remainingSize() < dataSize) + { + SAL_INFO("package", "Invalid Info-ZIP Unicode Path Extra Field: invalid TSize"); + throw ZipException("Invalid Info-ZIP Unicode Path Extra Field"); + } + auto const nVersion = aMemGrabber.ReadUInt8(); + if (nVersion != 1) + { + SAL_INFO("package", "Invalid Info-ZIP Unicode Path Extra Field: unexpected Version"); + throw ZipException("Invalid Info-ZIP Unicode Path Extra Field"); + } + // this CRC32 is actually of the pCENFilenameToCheck + // so it's pointless to check it if we require the UnicodeName + // to be equal to the CEN name anyway (and pCENFilenameToCheck + // is already converted to UTF-16 here) + (void) aMemGrabber.ReadUInt32(); + // this is required to be UTF-8 + OUString const unicodePath(reinterpret_cast<char const *>(aMemGrabber.getCurrentPos()), + dataSize - 5, RTL_TEXTENCODING_UTF8); + aMemGrabber.skipBytes(dataSize - 5); + if (unicodePath != *pCENFilenameToCheck) + { + SAL_INFO("package", "Invalid Info-ZIP Unicode Path Extra Field: unexpected UnicodeName"); + throw ZipException("Invalid Info-ZIP Unicode Path Extra Field"); + } + } + else + { + aMemGrabber.skipBytes(dataSize); + } + nExtraLen -= dataSize + 4; + } +} + void ZipFile::recover() { ::osl::MutexGuard aGuard( m_aMutexHolder->GetMutex() ); commit fb40e38e9b62b2bc6267b9807d19158d9659ca31 Author: Michael Stahl <[email protected]> AuthorDate: Tue Jul 2 11:47:17 2024 +0200 Commit: Michael Stahl <[email protected]> CommitDate: Mon Aug 19 16:25:52 2024 +0200 package: ZipFile: don't accept duplicate CEN entries Change-Id: Ice341a11346b445c555cba276bf2284e4f9b6685 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169881 Tested-by: Jenkins Reviewed-by: Michael Stahl <[email protected]> (cherry picked from commit 67cb6af482bc5b0f4d310f0a42a85057f3db0267) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169945 Reviewed-by: Caolán McNamara <[email protected]> (cherry picked from commit a4f85170623b40d33b914d95be109f6e8fe3c8fa) diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx index f693fa747ab0..4c36b5b767d5 100644 --- a/package/source/zipapi/ZipFile.cxx +++ b/package/source/zipapi/ZipFile.cxx @@ -979,7 +979,16 @@ sal_Int32 ZipFile::readCEN() throw ZipException("Zip entry has an invalid name." ); aMemGrabber.skipBytes( aEntry.nPathLen + aEntry.nExtraLen + nCommentLen ); - aEntries[aEntry.sPath] = aEntry; + + if (auto it = aEntries.find(aEntry.sPath); it == aEntries.end()) + { + aEntries[aEntry.sPath] = aEntry; + } + else + { + SAL_INFO("package", "Duplicate CEN entry: \"" << aEntry.sPath << "\""); + throw ZipException("Duplicate CEN entry"); + } } if (nCount != nTotal) commit 6ed53db1a50236a2543368731c3fe02cd2eefe80 Author: Caolán McNamara <[email protected]> AuthorDate: Tue Jan 2 12:31:01 2024 +0000 Commit: Michael Stahl <[email protected]> CommitDate: Mon Aug 19 16:25:52 2024 +0200 ofz#65480: Integer-overflow Change-Id: Ie771e6e37237907698e4c39eb50cd940500b86ca Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161540 Tested-by: Caolán McNamara <[email protected]> Reviewed-by: Caolán McNamara <[email protected]> (cherry picked from commit 69181dd430543db21fc0e61b70033ef484708c1c) diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx index 8111155de7e2..f693fa747ab0 100644 --- a/package/source/zipapi/ZipFile.cxx +++ b/package/source/zipapi/ZipFile.cxx @@ -32,6 +32,7 @@ #include <comphelper/processfactory.hxx> #include <rtl/digest.h> #include <sal/log.hxx> +#include <o3tl/safeint.hxx> #include <osl/diagnose.h> #include <algorithm> @@ -952,8 +953,10 @@ sal_Int32 ZipFile::readCEN() aEntry.nSize = nSize; aEntry.nOffset = nOffset; - aEntry.nOffset += nLocPos; - aEntry.nOffset *= -1; + if (o3tl::checked_add<sal_Int64>(aEntry.nOffset, nLocPos, aEntry.nOffset)) + throw ZipException("Integer-overflow"); + if (o3tl::checked_multiply<sal_Int64>(aEntry.nOffset, -1, aEntry.nOffset)) + throw ZipException("Integer-overflow"); if ( aEntry.nPathLen < 0 ) throw ZipException("unexpected name length" ); commit e85566adb52dd05b7c68085305b999f4f13cc3d9 Author: Caolán McNamara <[email protected]> AuthorDate: Tue Apr 25 12:33:26 2023 +0100 Commit: Michael Stahl <[email protected]> CommitDate: Mon Aug 19 16:25:52 2024 +0200 tdf#155005 fail gracefully on encountering a negative compression value we are using sal_Int64 for this so a large enough value can be interpreted as negative here Change-Id: Id547a24591aca4b6ed7b7955621641a0666b0bd5 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/150968 Tested-by: Caolán McNamara <[email protected]> Reviewed-by: Caolán McNamara <[email protected]> (cherry picked from commit 80805716a409c34203b059f3e03cd934367186c3) diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx index 7de1da7f2963..8111155de7e2 100644 --- a/package/source/zipapi/ZipFile.cxx +++ b/package/source/zipapi/ZipFile.cxx @@ -1175,6 +1175,12 @@ bool ZipFile::checkSizeAndCRC( const ZipEntry& aEntry ) if( aEntry.nMethod == STORED ) return ( getCRC( aEntry.nOffset, aEntry.nSize ) == aEntry.nCrc ); + if (aEntry.nCompressedSize < 0) + { + SAL_WARN("package", "bogus compressed size of: " << aEntry.nCompressedSize); + return false; + } + getSizeAndCRC( aEntry.nOffset, aEntry.nCompressedSize, &nSize, &nCRC ); return ( aEntry.nSize == nSize && aEntry.nCrc == nCRC ); } commit 4e016d64d7d20badea1c9b49818eb74271860653 Author: Michael Stahl <[email protected]> AuthorDate: Thu Jul 4 12:10:29 2024 +0200 Commit: Michael Stahl <[email protected]> CommitDate: Mon Aug 19 16:25:52 2024 +0200 sfx2: fix signature infobar being shown for every repaired document (regression from commit 8b333575ee680664fa3d83249ccec90881754ad7) So it should only be set if the state is still UNKNOWN. But SfxObjectShell::ImplGetSignatureState() is called before the repair dialog is shown, so make sure that the second import (with RepairPackage) finds both members as SignatureState::UNKOWN. Change-Id: Ic914016dde6425a4d95fba7f6f66411305553930 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169989 Tested-by: Jenkins Reviewed-by: Michael Stahl <[email protected]> (cherry picked from commit 8d869b5fe47842df52965804db87db0941f4f2a0) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169997 Reviewed-by: Caolán McNamara <[email protected]> (cherry picked from commit 127194ebbcbf644148fee81773babaf23eab78d8) diff --git a/sfx2/source/doc/objmisc.cxx b/sfx2/source/doc/objmisc.cxx index 635fabd3b292..9c0846a0f52a 100644 --- a/sfx2/source/doc/objmisc.cxx +++ b/sfx2/source/doc/objmisc.cxx @@ -964,6 +964,12 @@ void SfxObjectShell::BreakMacroSign_Impl( bool bBreakMacroSign ) void SfxObjectShell::CheckSecurityOnLoading_Impl() { + if (GetErrorCode() == ERRCODE_IO_BROKENPACKAGE) + { // safety first: don't run any macros from broken package. + pImpl->aMacroMode.disallowMacroExecution(); + return; // do not get signature status - needs to be done after RepairPackage + } + // make sure LO evaluates the macro signatures, so it can be preserved GetScriptingSignatureState(); diff --git a/sfx2/source/doc/objserv.cxx b/sfx2/source/doc/objserv.cxx index e769781396ca..9e06c74bcc70 100644 --- a/sfx2/source/doc/objserv.cxx +++ b/sfx2/source/doc/objserv.cxx @@ -1617,19 +1617,22 @@ SignatureState SfxObjectShell::ImplGetSignatureState( bool bScriptingContent ) { SignatureState* pState = bScriptingContent ? &pImpl->nScriptingSignatureState : &pImpl->nDocumentSignatureState; - // repaired package cannot be trusted - SfxBoolItem const*const pRepairItem{static_cast<SfxBoolItem const*>(GetMedium()->GetItemSet()->GetItem(SID_REPAIRPACKAGE, false))}; - if (pRepairItem && pRepairItem->GetValue()) - { - *pState = SignatureState::BROKEN; - } - if ( *pState == SignatureState::UNKNOWN ) { *pState = SignatureState::NOSIGNATURES; uno::Sequence< security::DocumentSignatureInformation > aInfos = GetDocumentSignatureInformation( bScriptingContent ); *pState = DocumentSignatures::getSignatureState(aInfos); + + // repaired package cannot be trusted + if (*pState != SignatureState::NOSIGNATURES) + { + SfxBoolItem const*const pRepairItem{static_cast<SfxBoolItem const*>(GetMedium()->GetItemSet()->GetItem(SID_REPAIRPACKAGE, false))}; + if (pRepairItem && pRepairItem->GetValue()) + { + *pState = SignatureState::BROKEN; + } + } } if ( *pState == SignatureState::OK || *pState == SignatureState::NOTVALIDATED diff --git a/sfx2/source/doc/objstor.cxx b/sfx2/source/doc/objstor.cxx index b232a286900d..1b9b1e026cd4 100644 --- a/sfx2/source/doc/objstor.cxx +++ b/sfx2/source/doc/objstor.cxx @@ -387,6 +387,8 @@ void SfxObjectShell::PrepareSecondTryLoad_Impl() { // only for internal use pImpl->m_xDocStorage.clear(); + pImpl->nDocumentSignatureState = SignatureState::UNKNOWN; + pImpl->nScriptingSignatureState = SignatureState::UNKNOWN; pImpl->m_bIsInit = false; ResetError(); } commit e4e9c810523702ef989ac4b03e23f57aedc72562 Author: Michael Stahl <[email protected]> AuthorDate: Tue Jul 2 13:24:38 2024 +0200 Commit: Michael Stahl <[email protected]> CommitDate: Fri Aug 16 18:18:38 2024 +0200 sfx2: SfxObjectShell should not trust any signature on repaired package Change-Id: I0317f80989e9dabd23e88e3caab26ede3fb5bd56 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169883 Tested-by: Jenkins Reviewed-by: Michael Stahl <[email protected]> (cherry picked from commit 8b333575ee680664fa3d83249ccec90881754ad7) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169930 Reviewed-by: Caolán McNamara <[email protected]> (cherry picked from commit 05b9e388448b1c8c10b18c22898c4725dd176fed) diff --git a/sfx2/source/doc/objserv.cxx b/sfx2/source/doc/objserv.cxx index 5c5b7826e075..e769781396ca 100644 --- a/sfx2/source/doc/objserv.cxx +++ b/sfx2/source/doc/objserv.cxx @@ -1617,6 +1617,13 @@ SignatureState SfxObjectShell::ImplGetSignatureState( bool bScriptingContent ) { SignatureState* pState = bScriptingContent ? &pImpl->nScriptingSignatureState : &pImpl->nDocumentSignatureState; + // repaired package cannot be trusted + SfxBoolItem const*const pRepairItem{static_cast<SfxBoolItem const*>(GetMedium()->GetItemSet()->GetItem(SID_REPAIRPACKAGE, false))}; + if (pRepairItem && pRepairItem->GetValue()) + { + *pState = SignatureState::BROKEN; + } + if ( *pState == SignatureState::UNKNOWN ) { *pState = SignatureState::NOSIGNATURES;
