Hi all,

attached to the e-mail is a patch which I propose to fix CVE-2017-5853 and 
maybe also CVE-2017-5855.
I haven't completed testing yet, but I have also already seen by running some 
test code, also attached,
that my patch catches the bogus xref subsection start in the reproducer PDF 
(linked from Agostino
Sarubbo's blog post). Please consider the patch for testing and applying it to 
the public repository
(besides line number changes, it should apply to all currently open branches: 
0.9.4, 0.9.5 and trunk.

Best regards, mabri
--- PdfParser.cpp  (revision 1830) 
+++ PdfParser.cpp  (working copy)
@@ -52,6 +52,7 @@
 #include <algorithm>
 #include <iostream>
 #include <limits>
+#include <climits>
 
 using std::cerr;
 using std::endl;
@@ -60,12 +61,13 @@ using std::flush;
 #define PDF_MAGIC_LEN       8
 #define PDF_XREF_ENTRY_SIZE 20
 #define PDF_XREF_BUF        512
-
 #if defined( PTRDIFF_MAX )
 #define PDF_LONG_MAX PTRDIFF_MAX
+#define PDF_MAX_INDIRECT_OBJECTS PTRDIFF_MAX/(INT_MAX/8388607) // see #else 
 #else
 // only old compilers don't define PTRDIFF_MAX (all 32-bit only?)
 #define PDF_LONG_MAX INT_MAX
+#define PDF_MAX_INDIRECT_OBJECTS 8388607 // from ISO 32000-1:2008 Appendix C.2
 #endif
 
 namespace PoDoFo {
@@ -745,30 +747,63 @@ void PdfParser::ReadXRefContents( pdf_lo
 
 void PdfParser::ReadXRefSubsection( pdf_int64 & nFirstObject, pdf_int64 & nNumObjects )
 {
-    int count = 0;
+    pdf_int64 count = 0;
 
 #ifdef PODOFO_VERBOSE_DEBUG
     PdfError::DebugMessage("Reading XRef Section: %" PDF_FORMAT_INT64 " with %" PDF_FORMAT_INT64 " Objects.\n", nFirstObject, nNumObjects );
 #endif // PODOFO_VERBOSE_DEBUG 
 
-    if ( nFirstObject + nNumObjects > m_nNumObjects )
+    const pdf_int64 maxNum
+      = static_cast<pdf_int64>(std::numeric_limits<long>::max());
+
+    if( (maxNum >= nNumObjects) && (nFirstObject < maxNum - nNumObjects) )
     {
-        // Total number of xref entries to read is greater than the /Size
-        // specified in the trailer if any. That's an error unless we're trying
-        // to recover from a missing /Size entry.
-		PdfError::LogMessage( eLogSeverity_Warning,
-			      "There are more objects (%" PDF_FORMAT_INT64 ") in this XRef table than "
-			      "specified in the size key of the trailer directory (%" PDF_FORMAT_INT64 ")!\n",
-			      nFirstObject + nNumObjects, m_nNumObjects );
+        if( nFirstObject + nNumObjects > m_nNumObjects )
+        {
+            // Total number of xref entries to read is greater than the /Size
+            // specified in the trailer if any. That's an error unless we're
+            // trying to recover from a missing /Size entry.
+            PdfError::LogMessage( eLogSeverity_Warning,
+              "\nThere are more objects (%" PDF_FORMAT_INT64 ") in this XRef "
+              "table than specified in the size key of the trailer directory "
+              "(%ld)!\n",
+              nFirstObject + nNumObjects, m_nNumObjects ); // 2nd arg is long!
 
-#ifdef _WIN32
-		m_nNumObjects = static_cast<long>(nFirstObject + nNumObjects);
-		m_offsets.resize(static_cast<long>(nFirstObject+nNumObjects));
-#else
-		m_nNumObjects = nFirstObject + nNumObjects;
-		m_offsets.resize(nFirstObject+nNumObjects);
-#endif // _WIN32
-	}
+            m_nNumObjects =  static_cast<long>(nFirstObject + nNumObjects);
+            m_offsets.resize(static_cast<long>(nFirstObject + nNumObjects));
+        }
+    }
+    else
+    {
+        pdf_int64 totalEntries;
+        bool totalWouldOverflow = false;
+
+        const pdf_int64 maxNum64 = std::numeric_limits<pdf_int64>::max();
+        if( (maxNum64 > nNumObjects)
+            && (nFirstObject < maxNum64 - nNumObjects) )
+            totalEntries = nFirstObject + nNumObjects;
+        else
+        {
+            totalEntries = maxNum64;
+            totalWouldOverflow = true;
+        }
+
+        std::string warning( "\nThere are more objects (%" PDF_FORMAT_INT64
+            ") in this XRef table than supported by this version of PoDoFo, "
+            "or it's inconsistent.\n");
+        PdfError::LogMessage( eLogSeverity_Warning, (warning +
+            (totalWouldOverflow ? "This caused the number to be underreported."
+            : "") + "\n").c_str(), totalEntries );
+        if( totalWouldOverflow || (totalEntries > PDF_MAX_INDIRECT_OBJECTS) )
+        {
+            PODOFO_RAISE_ERROR( ePdfError_InvalidXRef );
+        }
+        else
+        {
+            m_nNumObjects =  static_cast<long>(totalEntries);
+            m_offsets.resize(static_cast<long>(totalEntries));
+        }
+    }
 
     // consume all whitespaces
     int charcode;
#include <iostream>
#include "podofo/podofo.h"

int main()
{
    PoDoFo::PdfVecObjects testObjVec;
    PoDoFo::PdfParser p(&testObjVec, "00144-podofo-signintoverflow-PdfParser");
    std::cout << "object count in PoC: " << testObjVec.GetObjectCount();
    std::cout << std::endl;
    return 0;
}

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users

Reply via email to