comphelper/source/misc/storagehelper.cxx | 13
package/inc/ByteGrabber.hxx | 1
package/inc/ZipFile.hxx | 27 -
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 | 555 +++++++++++++++++++++++++---
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 | 5
15 files changed, 666 insertions(+), 77 deletions(-)
New commits:
commit 72a0f94d49764bc1298d721019b7313ec7a0e1ab
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Tue Aug 13 17:59:18 2024 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Aug 19 14:55:34 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>
(cherry picked from commit 32cad89592ec04ab552399095c91dd76afb3002c)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171779
Reviewed-by: Adolfo Jayme Barrientos <fit...@ubuntu.com>
(cherry picked from commit 525fe1a2e58a4d6164aa104e893d4f07a5709b27)
diff --git a/package/inc/ZipPackage.hxx b/package/inc/ZipPackage.hxx
index e6cf614ad503..5784bed6e5bf 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 d7a6633a4266..16ae81e2e40e 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -882,7 +882,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;
@@ -1270,6 +1274,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 48ecb3aa4209..b353a0caf9d0 100644
--- a/package/source/zippackage/ZipPackage.cxx
+++ b/package/source/zippackage/ZipPackage.cxx
@@ -158,6 +158,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 )
@@ -352,6 +377,8 @@ void ZipPackage::parseManifest()
bManifestParsed = true;
}
+ checkZipEntriesWithDD(); // check before removing entries!
+
// now hide the manifest.xml file from user
xMetaInfFolder->removeByName( sManifest );
}
@@ -583,7 +610,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 f52d1148d185b58005289a7d7ab29226d9989df2
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Fri Jul 19 20:43:05 2024 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Aug 19 14:55:34 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 <michael.st...@allotropia.de>
Tested-by: Jenkins
(cherry picked from commit ca21cc985d57fffe7c834159b17c095206304994)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/171209
Reviewed-by: Adolfo Jayme Barrientos <fit...@ubuntu.com>
(cherry picked from commit 1d37c37ab6025bf5b107cc0c18eae5739ff02149)
diff --git a/package/inc/ZipFile.hxx b/package/inc/ZipFile.hxx
index c431aa23043d..f633846a3b30 100644
--- a/package/inc/ZipFile.hxx
+++ b/package/inc/ZipFile.hxx
@@ -90,7 +90,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/MemoryByteGrabber.hxx
b/package/source/zipapi/MemoryByteGrabber.hxx
index 650b5c6a6d02..dd876d66ebfa 100644
--- a/package/source/zipapi/MemoryByteGrabber.hxx
+++ b/package/source/zipapi/MemoryByteGrabber.hxx
@@ -99,6 +99,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 7bbdfbe593f5..d7a6633a4266 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -978,18 +978,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 nPos, nEnd;
Sequence < sal_Int8 > aBuffer;
try
{
- sal_Int32 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 );
@@ -999,13 +1000,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& )
{
@@ -1019,41 +1152,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, nLocPos;
- sal_uInt16 nCount;
+ sal_Int32 nCenPos = -1;
try
{
- sal_Int32 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 )
@@ -1066,6 +1188,7 @@ sal_Int32 ZipFile::readCEN()
::std::vector<std::pair<sal_uInt64, sal_uInt64>> unallocated = { { 0,
nCenPos } };
aEntries.reserve(nTotal);
+ sal_Int64 nCount;
for (nCount = 0 ; nCount < nTotal; nCount++)
{
sal_Int32 nTestSig = aMemGrabber.ReadInt32();
@@ -1105,8 +1228,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 f3b68860aaddf77f454a70c1bcc5c494f37550e4
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Tue Jul 16 12:12:09 2024 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Aug 19 14:55:34 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 <michael.st...@allotropia.de>
(cherry picked from commit efae4fc42d5fe3c0a69757226f38efc10d101194)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170588
Reviewed-by: Thorsten Behrens <thorsten.behr...@allotropia.de>
(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...
diff --git a/package/inc/ByteGrabber.hxx b/package/inc/ByteGrabber.hxx
index ba1512cf5162..4a93398a7704 100644
--- a/package/inc/ByteGrabber.hxx
+++ b/package/inc/ByteGrabber.hxx
@@ -61,6 +61,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 0d96abe3d067..c431aa23043d 100644
--- a/package/inc/ZipFile.hxx
+++ b/package/inc/ZipFile.hxx
@@ -29,6 +29,7 @@
#include "HashMaps.hxx"
#include "EncryptionData.hxx"
+#include <optional>
#include <unordered_set>
class MemoryByteGrabber;
@@ -87,13 +88,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 b58a7087f468..19b5a4cc9755 100644
--- a/package/source/zipapi/ByteGrabber.cxx
+++ b/package/source/zipapi/ByteGrabber.cxx
@@ -122,4 +122,22 @@ sal_uInt32 ByteGrabber::ReadUInt32()
| ( pSequence[3] & 0xFF ) << 24 );
}
+sal_uInt64 ByteGrabber::ReadUInt64()
+{
+ std::scoped_lock 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/ZipFile.cxx
b/package/source/zipapi/ZipFile.cxx
index d79ce545aaf3..7bbdfbe593f5 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -772,7 +772,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() );
@@ -788,19 +788,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
@@ -828,8 +832,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(...)
{
@@ -838,6 +974,8 @@ void ZipFile::readLOC( ZipEntry &rEntry )
if ( bBroken && !bRecoveryMode )
throw ZipIOException("The stream seems to be broken!" );
+
+ return nEnd;
}
sal_Int32 ZipFile::findEND()
@@ -925,7 +1063,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 } };
aEntries.reserve(nTotal);
for (nCount = 0 ; nCount < nTotal; nCount++)
@@ -969,7 +1107,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");
@@ -997,11 +1134,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
<< "\"");
@@ -1019,9 +1221,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 & )
@@ -1032,37 +1234,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)
{
@@ -1096,6 +1309,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 44dc5ace5ad7..c811d0261354 100644
--- a/sc/qa/unit/filters-test.cxx
+++ b/sc/qa/unit/filters-test.cxx
@@ -32,6 +32,12 @@
#include <svx/svdocapt.hxx>
#include <svx/svdpage.hxx>
#include <tools/stream.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;
@@ -874,8 +880,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_ASSERTION_FAIL(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/sd/qa/unit/data/pptx/pass/ofz35597-1.pptx
b/sd/qa/unit/data/pptx/fail/ofz35597-1.pptx
similarity index 100%
rename from sd/qa/unit/data/pptx/pass/ofz35597-1.pptx
rename to sd/qa/unit/data/pptx/fail/ofz35597-1.pptx
diff --git a/sd/qa/unit/data/pptx/pass/ofz46160-1.pptx
b/sd/qa/unit/data/pptx/fail/ofz46160-1.pptx
similarity index 100%
rename from sd/qa/unit/data/pptx/pass/ofz46160-1.pptx
rename to sd/qa/unit/data/pptx/fail/ofz46160-1.pptx
diff --git a/vcl/qa/cppunit/pdfexport/pdfexport.cxx
b/vcl/qa/cppunit/pdfexport/pdfexport.cxx
index 0d67999b25f7..69c02c7218a5 100644
--- a/vcl/qa/cppunit/pdfexport/pdfexport.cxx
+++ b/vcl/qa/cppunit/pdfexport/pdfexport.cxx
@@ -1783,8 +1783,9 @@ CPPUNIT_TEST_FIXTURE(PdfExportTest, testTdf113143)
CPPUNIT_TEST_FIXTURE(PdfExportTest, testForcePoint71)
{
// I just care it doesn't crash
- aMediaDescriptor["FilterName"] <<= OUString("writer_pdf_Export");
- saveAsPDF(u"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));
}
CPPUNIT_TEST_FIXTURE(PdfExportTest, testForcePoint80)
commit c0ac29e1b68a92f220e1c1d3d018bfe2c825ad27
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Wed Jul 17 12:04:13 2024 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Aug 19 14:55:34 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 <michael.st...@allotropia.de>
(cherry picked from commit 0fb25ce9ff9a3ede8d43ee1502c44b4c02135b3f)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170623
Reviewed-by: Xisco Fauli <xiscofa...@libreoffice.org>
(cherry picked from commit bf9ece7cf6d184675fac1eba88ebcb04bf81442f)
diff --git a/package/inc/ZipFile.hxx b/package/inc/ZipFile.hxx
index 9490a6c592de..0d96abe3d067 100644
--- a/package/inc/ZipFile.hxx
+++ b/package/inc/ZipFile.hxx
@@ -52,9 +52,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;
@@ -93,16 +98,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 d5d35087ed95..d79ce545aaf3 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -79,27 +79,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)
@@ -1026,7 +1009,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 3d6a75ad55d5..48ecb3aa4209 100644
--- a/package/source/zippackage/ZipPackage.cxx
+++ b/package/source/zippackage/ZipPackage.cxx
@@ -762,7 +762,9 @@ void SAL_CALL ZipPackage::initialize( const uno::Sequence<
Any >& aArguments )
OUString message;
try
{
- m_pZipFile.emplace(m_aMutexHolder, m_xContentStream, m_xContext, true,
m_bForceRecovery);
+ m_pZipFile.emplace(m_aMutexHolder, m_xContentStream, m_xContext, true,
+ m_bForceRecovery,
+ m_nFormat == embed::StorageFormats::ZIP ? ZipFile::Checks::Default
: ZipFile::Checks::CheckInsensitive);
getZipFileContents();
}
catch ( IOException & e )
@@ -1135,7 +1137,9 @@ void ZipPackage::ConnectTo( const uno::Reference<
io::XInputStream >& xInStream
if ( m_pZipFile )
m_pZipFile->setInputStream( m_xContentStream );
else
- m_pZipFile.emplace(m_aMutexHolder, m_xContentStream, m_xContext,
false);
+ m_pZipFile.emplace(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 d9b17fd80b49..1e185a21b280 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 8dc00d5db2389fc405f7ae6620b6209fe0c34207
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Tue Jul 9 17:22:15 2024 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Aug 19 14:55:34 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 <michael.st...@allotropia.de>
(cherry picked from commit 4833f131243bdb409ddfaff8b4db87d4ed2af98f)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170282
Reviewed-by: Christian Lohmaier <lohmaier+libreoff...@googlemail.com>
(cherry picked from commit 92564a17fe68066f6e9135c6f79fb6319b192f22)
diff --git a/package/inc/ZipFile.hxx b/package/inc/ZipFile.hxx
index d141df934d8c..9490a6c592de 100644
--- a/package/inc/ZipFile.hxx
+++ b/package/inc/ZipFile.hxx
@@ -29,6 +29,8 @@
#include "HashMaps.hxx"
#include "EncryptionData.hxx"
+#include <unordered_set>
+
class MemoryByteGrabber;
namespace com::sun::star {
namespace uno { class XComponentContext; }
@@ -52,6 +54,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 8ecd6d5109d5..d5d35087ed95 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -90,6 +90,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" );
}
}
@@ -114,6 +115,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" );
}
}
@@ -1017,15 +1019,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)
@@ -1200,6 +1206,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 09ad54e955dbb291d805fafbd8ef90b315a35861
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Fri Jul 5 13:55:31 2024 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Aug 19 14:55:34 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 <michael.st...@allotropia.de>
(cherry picked from commit c409c83d777fdb6291c7cd03186b69fe4e7fd902)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170030
Reviewed-by: Xisco Fauli <xiscofa...@libreoffice.org>
(cherry picked from commit bcb6130ef8ab66a99347945c1c30ec0112d6940e)
diff --git a/package/source/zippackage/ZipPackage.cxx
b/package/source/zippackage/ZipPackage.cxx
index 9270bb70502e..3d6a75ad55d5 100644
--- a/package/source/zippackage/ZipPackage.cxx
+++ b/package/source/zippackage/ZipPackage.cxx
@@ -544,7 +544,11 @@ void ZipPackage::getZipFileContents()
if ( !pCurrent->hasByName( sTemp ) )
{
rtl::Reference<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.get();
}
commit bf1df9bed791580b88cb828a1e0a7ead1c59e625
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Thu Jul 4 17:52:49 2024 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Aug 19 14:55:34 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 <michael.st...@allotropia.de>
(cherry picked from commit 2afdc61dd3138b383fb73dae2242ba1a9c8de901)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170009
Reviewed-by: Xisco Fauli <xiscofa...@libreoffice.org>
(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 d353ecebce29..8ecd6d5109d5 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -940,6 +940,7 @@ sal_Int32 ZipFile::readCEN()
ZipEntry aEntry;
sal_Int16 nCommentLen;
+ sal_Int64 nMinOffset{nEndPos};
aEntries.reserve(nTotal);
for (nCount = 0 ; nCount < nTotal; nCount++)
@@ -983,6 +984,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");
@@ -1028,6 +1030,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 ab6b1f274fc93d83bed393983b8d5f08d7bd7d77
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Thu Jul 4 14:07:25 2024 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Aug 19 14:55:34 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 <michael.st...@allotropia.de>
Tested-by: Jenkins
(cherry picked from commit 6005260078c126bf3f1cf4d6f1ebb631453f5ac7)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169964
Reviewed-by: Xisco Fauli <xiscofa...@libreoffice.org>
(cherry picked from commit fda9ae6f1cef7512f0560828f67553b91641bd26)
diff --git a/comphelper/source/misc/storagehelper.cxx
b/comphelper/source/misc/storagehelper.cxx
index 5f8527bcf273..ef310d9837ee 100644
--- a/comphelper/source/misc/storagehelper.cxx
+++ b/comphelper/source/misc/storagehelper.cxx
@@ -565,10 +565,17 @@ bool OStorageHelper::IsValidZipEntryFileName(
std::u16string_view aName, bool bS
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 '<':
@@ -578,15 +585,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 3dcf350cb1c4bc78170c2412e46a4f5acb53087d
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Tue Jul 2 11:50:09 2024 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Aug 19 14:55:34 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 <michael.st...@allotropia.de>
(cherry picked from commit 67dca5a47d1be1b38edee7d0d81fa9adfce5a22e)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169929
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
(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 <caol...@redhat.com>
(cherry picked from commit 59b0f676758dd752457c84fb4159f6446d74e8a4)
Change-Id: I11d2247004e3fb20fe2e443f0d4aa755290aa667
diff --git a/package/inc/ZipFile.hxx b/package/inc/ZipFile.hxx
index 15b800430555..d141df934d8c 100644
--- a/package/inc/ZipFile.hxx
+++ b/package/inc/ZipFile.hxx
@@ -29,6 +29,7 @@
#include "HashMaps.hxx"
#include "EncryptionData.hxx"
+class MemoryByteGrabber;
namespace com::sun::star {
namespace uno { class XComponentContext; }
namespace ucb { class XProgressHandler; }
@@ -81,6 +82,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 8dcf7f067064..650b5c6a6d02 100644
--- a/package/source/zipapi/MemoryByteGrabber.hxx
+++ b/package/source/zipapi/MemoryByteGrabber.hxx
@@ -49,6 +49,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()
{
@@ -58,6 +66,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 931e9acc33c7..d353ecebce29 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -36,6 +36,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>
@@ -1005,7 +1006,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())
{
@@ -1029,6 +1037,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 2848cc5b7429e9a7536af4be33fef71e58a756b3
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Tue Jul 2 11:47:17 2024 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Aug 19 14:55:33 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 <michael.st...@allotropia.de>
(cherry picked from commit 67cb6af482bc5b0f4d310f0a42a85057f3db0267)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169945
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
(cherry picked from commit a4f85170623b40d33b914d95be109f6e8fe3c8fa)
diff --git a/package/source/zipapi/ZipFile.cxx
b/package/source/zipapi/ZipFile.cxx
index 18bd6f34d888..931e9acc33c7 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -1006,7 +1006,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 12941b0338df32c739b9fdb55c71210f3d1a779a
Author: Caolán McNamara <caolan.mcnam...@collabora.com>
AuthorDate: Tue Jan 2 12:31:01 2024 +0000
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Aug 19 14:55:33 2024 +0200
ofz#65480: Integer-overflow
Change-Id: Ie771e6e37237907698e4c39eb50cd940500b86ca
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/161540
Tested-by: Caolán McNamara <caolan.mcnam...@collabora.com>
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
(cherry picked from commit 69181dd430543db21fc0e61b70033ef484708c1c)
diff --git a/package/source/zipapi/ZipFile.cxx
b/package/source/zipapi/ZipFile.cxx
index 410d9b892ac4..18bd6f34d888 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -980,8 +980,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 15338b93c2f26936b0133ce65177b36a75754e25
Author: Caolán McNamara <caol...@redhat.com>
AuthorDate: Tue Apr 25 12:33:26 2023 +0100
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Aug 19 14:55:33 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 <caol...@redhat.com>
Reviewed-by: Caolán McNamara <caol...@redhat.com>
(cherry picked from commit 80805716a409c34203b059f3e03cd934367186c3)
diff --git a/package/source/zipapi/ZipFile.cxx
b/package/source/zipapi/ZipFile.cxx
index 79dc5bb68a8e..410d9b892ac4 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -1202,6 +1202,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 e6e35bedbc987866d9b8b6156daee493f46c737d
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Thu Jul 4 12:10:29 2024 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Mon Aug 19 14:55:33 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 <michael.st...@allotropia.de>
(cherry picked from commit 8d869b5fe47842df52965804db87db0941f4f2a0)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169997
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
(cherry picked from commit 127194ebbcbf644148fee81773babaf23eab78d8)
diff --git a/sfx2/source/doc/objmisc.cxx b/sfx2/source/doc/objmisc.cxx
index 86f587c20bec..17a6037520de 100644
--- a/sfx2/source/doc/objmisc.cxx
+++ b/sfx2/source/doc/objmisc.cxx
@@ -937,6 +937,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 11f07479e70f..47647ae5b231 100644
--- a/sfx2/source/doc/objserv.cxx
+++ b/sfx2/source/doc/objserv.cxx
@@ -1757,19 +1757,22 @@ SignatureState SfxObjectShell::ImplGetSignatureState(
bool bScriptingContent )
{
SignatureState* pState = bScriptingContent ?
&pImpl->nScriptingSignatureState : &pImpl->nDocumentSignatureState;
- // repaired package cannot be trusted
- SfxBoolItem const*const
pRepairItem{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{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 288b6a7ae6a9..5dcc38e42a0d 100644
--- a/sfx2/source/doc/objstor.cxx
+++ b/sfx2/source/doc/objstor.cxx
@@ -378,6 +378,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 ab3f0d7118e4fc086763365e3aa0596418cd83e5
Author: Michael Stahl <michael.st...@allotropia.de>
AuthorDate: Tue Jul 2 13:24:38 2024 +0200
Commit: Michael Stahl <michael.st...@allotropia.de>
CommitDate: Thu Aug 15 18:38:55 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 <michael.st...@allotropia.de>
(cherry picked from commit 8b333575ee680664fa3d83249ccec90881754ad7)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/169930
Reviewed-by: Caolán McNamara <caolan.mcnam...@collabora.com>
(cherry picked from commit 05b9e388448b1c8c10b18c22898c4725dd176fed)
diff --git a/sfx2/source/doc/objserv.cxx b/sfx2/source/doc/objserv.cxx
index 3a6a0ceea2a8..11f07479e70f 100644
--- a/sfx2/source/doc/objserv.cxx
+++ b/sfx2/source/doc/objserv.cxx
@@ -1757,6 +1757,13 @@ SignatureState SfxObjectShell::ImplGetSignatureState(
bool bScriptingContent )
{
SignatureState* pState = bScriptingContent ?
&pImpl->nScriptingSignatureState : &pImpl->nDocumentSignatureState;
+ // repaired package cannot be trusted
+ SfxBoolItem const*const
pRepairItem{GetMedium()->GetItemSet()->GetItem(SID_REPAIRPACKAGE, false)};
+ if (pRepairItem && pRepairItem->GetValue())
+ {
+ *pState = SignatureState::BROKEN;
+ }
+
if ( *pState == SignatureState::UNKNOWN )
{
*pState = SignatureState::NOSIGNATURES;
