From ca21cc985d57fffe7c834159b17c095206304994 Mon Sep 17 00:00:00 2001 From: Michael Stahl Date: Fri, 19 Jul 2024 20:43:05 +0200 Subject: 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 Tested-by: Jenkins --- package/inc/ZipFile.hxx | 2 +- package/source/zipapi/ZipFile.cxx | 181 +++++++++++++++++++++++++++++++------- 2 files changed, 152 insertions(+), 31 deletions(-) (limited to 'package') diff --git a/package/inc/ZipFile.hxx b/package/inc/ZipFile.hxx index 491309d9bd68..65df2ad00a60 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 findCentralDirectory(); void HandlePK34(std::span data, sal_Int64 dataOffset, sal_Int64 totalSize); void HandlePK78(std::span data, sal_Int64 dataOffset); void recover(); diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx index b83d52cfae79..6c5054e19d10 100644 --- a/package/source/zipapi/ZipFile.cxx +++ b/package/source/zipapi/ZipFile.cxx @@ -1109,18 +1109,19 @@ sal_uInt64 ZipFile::readLOC(ZipEntry &rEntry) return nEnd; } -sal_Int32 ZipFile::findEND() +std::tuple 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 (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(u"Zip too small!"_ustr); + } + sal_Int64 nPos = nLength - ENDHDR - ZIP_MAXNAMELEN; + sal_Int64 nEnd = nPos >= 0 ? nPos : 0; aGrabber.seek( nEnd ); @@ -1130,13 +1131,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(u"Zip END signature not found!"_ustr); + } + } + + aGrabber.seek(nEndPos + 4); + sal_uInt16 const nEndDisk = aGrabber.ReadUInt16(); + if (nEndDisk != 0) + { // only single disk is supported! + throw ZipException(u"invalid end (disk)"_ustr ); + } + sal_uInt16 const nEndDirDisk = aGrabber.ReadUInt16(); + if (nEndDirDisk != 0) + { + throw ZipException(u"invalid end (directory disk)"_ustr ); + } + sal_uInt16 const nEndDiskEntries = aGrabber.ReadUInt16(); + sal_uInt16 const nEndEntries = aGrabber.ReadUInt16(); + if (nEndDiskEntries != nEndEntries) + { + throw ZipException(u"invalid end (entries)"_ustr ); + } + 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 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(u"invalid Zip64 end locator (disk)"_ustr); + } + sal_Int64 const nLoc64End64Offset = loc64Grabber.ReadUInt64(); + if (nEndPos < 20 + 56 || (nEndPos - 20 - 56) < nLoc64End64Offset + || nLoc64End64Offset < 0) + { + throw ZipException(u"invalid Zip64 end locator (offset)"_ustr); + } + sal_uInt32 const nLoc64Disks = loc64Grabber.ReadUInt32(); + if (nLoc64Disks != 1) + { + throw ZipException(u"invalid Zip64 end locator (number of disks)"_ustr); + } + aGrabber.seek(nLoc64End64Offset); + Sequence 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(u"invalid Zip64 end (signature)"_ustr); + } + sal_Int64 const nEnd64Size = end64Grabber.ReadUInt64(); + if (nEnd64Size != nEndPos - 20 - nLoc64End64Offset - 12) + { + throw ZipException(u"invalid Zip64 end (size)"_ustr); + } + end64Grabber.ReadUInt16(); // ignore version made by + end64Grabber.ReadUInt16(); // ignore version needed to extract + sal_uInt32 const nEnd64Disk = end64Grabber.ReadUInt32(); + if (nEnd64Disk != 0) + { + throw ZipException(u"invalid Zip64 end (disk)"_ustr); + } + sal_uInt32 const nEnd64EndDisk = end64Grabber.ReadUInt32(); + if (nEnd64EndDisk != 0) + { + throw ZipException(u"invalid Zip64 end (directory disk)"_ustr); + } + sal_uInt64 const nEnd64DiskEntries = end64Grabber.ReadUInt64(); + sal_uInt64 const nEnd64Entries = end64Grabber.ReadUInt64(); + if (nEnd64DiskEntries != nEnd64Entries) + { + throw ZipException(u"invalid Zip64 end (entries)"_ustr); + } + sal_Int64 const nEnd64DirSize = end64Grabber.ReadUInt64(); + sal_Int64 const nEnd64DirOffset = end64Grabber.ReadUInt64(); + if (nEndEntries != sal_uInt16(-1) && nEnd64Entries != nEndEntries) + { + throw ZipException(u"inconsistent Zip/Zip64 end (entries)"_ustr); + } + if (o3tl::make_unsigned(nEndDirSize) != sal_uInt32(-1) + && nEnd64DirSize != nEndDirSize) + { + throw ZipException(u"inconsistent Zip/Zip64 end (size)"_ustr); + } + if (o3tl::make_unsigned(nEndDirOffset) != sal_uInt32(-1) + && nEnd64DirOffset != nEndDirOffset) + { + throw ZipException(u"inconsistent Zip/Zip64 end (offset)"_ustr); + } + + sal_Int64 end; + if (o3tl::checked_add(nEnd64DirOffset, nEnd64DirSize, end) + || nLoc64End64Offset < end + || nEnd64DirOffset < 0 + || nLoc64End64Offset - nEnd64DirSize != nEnd64DirOffset) + { + throw ZipException(u"Invalid Zip64 end (bad central directory size)"_ustr); + } + + return { nEnd64Entries, nEnd64DirSize, nEnd64DirOffset }; + } } + + sal_Int32 end; + if (o3tl::checked_add(nEndDirOffset, nEndDirSize, end) + || nEndPos < end + || nEndDirOffset < 0 + || nEndPos - nEndDirSize != nEndDirOffset) + { + throw ZipException(u"Invalid END header (bad central directory size)"_ustr); + } + + return { nEndEntries, nEndDirSize, nEndDirOffset }; } catch ( IllegalArgumentException& ) { @@ -1150,41 +1283,30 @@ sal_Int32 ZipFile::findEND() { throw ZipException(u"Zip END signature not found!"_ustr ); } - throw ZipException(u"Zip END signature not found!"_ustr ); } 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(u"invalid END header (bad entry count)"_ustr ); + auto [nTotal, nCenLen, nCenOff] = findCentralDirectory(); + nCenPos = nCenOff; // data before start of zip is not supported if ( nTotal > ZIP_MAXENTRIES ) throw ZipException(u"too many entries in ZIP File"_ustr ); - if ( nCenLen < 0 || nCenLen > nEndPos ) - throw ZipException(u"Invalid END header (bad central directory size)"_ustr ); - - nCenPos = nEndPos - nCenLen; + if (nCenLen < nTotal * CENHDR) // prevent overflow with ZIP_MAXENTRIES + throw ZipException(u"invalid END header (bad entry count)"_ustr ); - if ( nCenOff < 0 || nCenOff > nCenPos ) - throw ZipException(u"Invalid END header (bad central directory size)"_ustr ); + if (SAL_MAX_INT32 < nCenLen) + { + throw ZipException(u"central directory too big"_ustr); + } - 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 ) @@ -1197,6 +1319,7 @@ sal_Int32 ZipFile::readCEN() ::std::vector> unallocated = { { 0, nCenPos } }; aEntries.reserve(nTotal); + sal_Int64 nCount; for (nCount = 0 ; nCount < nTotal; nCount++) { sal_Int32 nTestSig = aMemGrabber.ReadInt32(); @@ -1262,8 +1385,6 @@ sal_Int32 ZipFile::readCEN() aEntry.nSize = nSize; aEntry.nOffset = nOffset; - if (o3tl::checked_add(aEntry.nOffset, nLocPos, aEntry.nOffset)) - throw ZipException(u"Integer-overflow"_ustr); if (o3tl::checked_multiply(aEntry.nOffset, -1, aEntry.nOffset)) throw ZipException(u"Integer-overflow"_ustr); -- cgit v1.2.3