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.stahl@allotropia.de>
Tested-by: Jenkins

2 files changed, 152 insertions, 31 deletions

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<sal_Int64, sal_Int64, sal_Int64> findCentralDirectory();
     void HandlePK34(std::span<const sal_Int8> data, sal_Int64 dataOffset, sal_Int64 totalSize);
     void HandlePK78(std::span<const sal_Int8> 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<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(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<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(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<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(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<sal_Int64>(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<sal_Int32>(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<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();
@@ -1262,8 +1385,6 @@ sal_Int32 ZipFile::readCEN()
             aEntry.nSize = nSize;
             aEntry.nOffset = nOffset;
 
-            if (o3tl::checked_add<sal_Int64>(aEntry.nOffset, nLocPos, aEntry.nOffset))
-                throw ZipException(u"Integer-overflow"_ustr);
             if (o3tl::checked_multiply<sal_Int64>(aEntry.nOffset, -1, aEntry.nOffset))
                 throw ZipException(u"Integer-overflow"_ustr);