Fix infinite recursion in CPDF_Parser::ParseIndirectObjectAt().
authorTom Sepez <tsepez@chromium.org>
Tue, 27 Jan 2015 00:51:21 +0000 (16:51 -0800)
committerTom Sepez <tsepez@chromium.org>
Tue, 27 Jan 2015 00:51:21 +0000 (16:51 -0800)
A suitably corrupted file can cause the parser(s) to repeatedly re-read
sections of the file at increasing parser recursion depth until the
stack is exhausted.  There is supposed to be a check for this based upon
the parser "level", but not all call paths pass or update the level as
required.

Much as I hate per-class statics, this introduces one to track the depth
so that the check is enforced no matter how screwy the call path might be
that leads the parser to re-enter itself. This is more palatable than trying
to find all these paths and fix them. We know this is OK since there is
only one thread in here modifying the static.

BUG=451830
R=thestig@chromium.org

Review URL: https://codereview.chromium.org/875263002

core/include/fpdfapi/fpdf_parser.h
core/include/fxcrt/fx_basic.h
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
fpdfsdk/src/fpdfview_embeddertest.cpp
testing/resources/bug_451830.pdf [new file with mode: 0644]

index 3f9bda5..e1a0f5c 100644 (file)
@@ -261,10 +261,10 @@ public:
         m_Pos = pos;
     }
 
-    CPDF_Object*               GetObject(CPDF_IndirectObjects* pObjList, FX_DWORD objnum, FX_DWORD gennum, int level, struct PARSE_CONTEXT* pContext = NULL, FX_BOOL bDecrypt = TRUE);
+    CPDF_Object*               GetObject(CPDF_IndirectObjects* pObjList, FX_DWORD objnum, FX_DWORD gennum, struct PARSE_CONTEXT* pContext = NULL, FX_BOOL bDecrypt = TRUE);
 
 
-    CPDF_Object*               GetObjectByStrict(CPDF_IndirectObjects* pObjList, FX_DWORD objnum, FX_DWORD gennum, int level, struct PARSE_CONTEXT* pContext = NULL);
+    CPDF_Object*               GetObjectByStrict(CPDF_IndirectObjects* pObjList, FX_DWORD objnum, FX_DWORD gennum, struct PARSE_CONTEXT* pContext = NULL);
 
     int                                        GetDirectNum();
 
@@ -302,6 +302,8 @@ public:
 
     CFX_ByteString             GetNextWord(FX_BOOL& bIsNumber);
 protected:
+    static const int kParserMaxRecursionDepth = 64;
+    static int s_CurrentRecursionDepth;
 
     virtual FX_BOOL                            GetNextChar(FX_BYTE& ch);
 
@@ -520,7 +522,6 @@ public:
         return m_dwFirstPageNo;
     }
 protected:
-
     CPDF_Document*             m_pDocument;
 
     CPDF_SyntaxParser  m_Syntax;
index 62324f5..7ad44c6 100644 (file)
@@ -1414,6 +1414,21 @@ protected:
 
     CFX_DataFilter*    m_pDestFilter;
 };
+
+template<typename T>
+class CFX_AutoRestorer {
+public:
+    explicit CFX_AutoRestorer(T* location) {
+      m_Location = location;
+      m_OldValue = *location;
+    }
+    ~CFX_AutoRestorer() { *m_Location = m_OldValue; }
+
+private:
+  T* m_Location;
+  T m_OldValue;
+};
+
 template <class T>
 class CFX_SmartPointer
 {
index 5dfcc82..8195989 100644 (file)
@@ -12,7 +12,6 @@
 #include <utility>
 #include <vector>
 
-#define _PARSER_OBJECT_LEVLE_          64
 extern const FX_LPCSTR _PDF_CharType;
 FX_BOOL IsSignatureDict(const CPDF_Dictionary* pDict)
 {
@@ -39,6 +38,7 @@ static int _CompareFileSize(const void* p1, const void* p2)
     }
     return 0;
 }
+
 CPDF_Parser::CPDF_Parser()
 {
     m_pDocument = NULL;
@@ -1217,7 +1217,7 @@ CPDF_Object* CPDF_Parser::ParseIndirectObject(CPDF_IndirectObjects* pObjList, FX
             FX_DWORD thisoff = syntax.GetDirectNum();
             if (thisnum == objnum) {
                 syntax.RestorePos(offset + thisoff);
-                pRet = syntax.GetObject(pObjList, 0, 0, 0, pContext);
+                pRet = syntax.GetObject(pObjList, 0, 0, pContext);
                 break;
             }
             n --;
@@ -1396,7 +1396,7 @@ CPDF_Object* CPDF_Parser::ParseIndirectObjectAt(CPDF_IndirectObjects* pObjList,
         m_Syntax.RestorePos(SavedPos);
         return NULL;
     }
-    CPDF_Object* pObj = m_Syntax.GetObject(pObjList, objnum, parser_gennum, 0, pContext);
+    CPDF_Object* pObj = m_Syntax.GetObject(pObjList, objnum, parser_gennum, pContext);
     FX_FILESIZE endOffset = m_Syntax.SavePos();
     CFX_ByteString bsWord = m_Syntax.GetKeyword();
     if (bsWord == FX_BSTRC("endobj")) {
@@ -1437,7 +1437,7 @@ CPDF_Object* CPDF_Parser::ParseIndirectObjectAtByStrict(CPDF_IndirectObjects* pO
         m_Syntax.RestorePos(SavedPos);
         return NULL;
     }
-    CPDF_Object* pObj = m_Syntax.GetObjectByStrict(pObjList, objnum, gennum, 0, pContext);
+    CPDF_Object* pObj = m_Syntax.GetObjectByStrict(pObjList, objnum, gennum, pContext);
     if (pResultPos) {
         *pResultPos = m_Syntax.m_Pos;
     }
@@ -1676,6 +1676,10 @@ FX_DWORD CPDF_Parser::LoadLinearizedMainXRefTable()
     m_Syntax.m_MetadataObjnum = dwSaveMetadataObjnum;
     return PDFPARSE_ERROR_SUCCESS;
 }
+
+// static
+int CPDF_SyntaxParser::s_CurrentRecursionDepth = 0;
+
 CPDF_SyntaxParser::CPDF_SyntaxParser()
 {
     m_pFileAccess = NULL;
@@ -2049,9 +2053,10 @@ CFX_ByteString CPDF_SyntaxParser::GetKeyword()
     GetNextWord();
     return CFX_ByteString((FX_LPCSTR)m_WordBuffer, m_WordSize);
 }
-CPDF_Object* CPDF_SyntaxParser::GetObject(CPDF_IndirectObjects* pObjList, FX_DWORD objnum, FX_DWORD gennum, FX_INT32 level, PARSE_CONTEXT* pContext, FX_BOOL bDecrypt)
+CPDF_Object* CPDF_SyntaxParser::GetObject(CPDF_IndirectObjects* pObjList, FX_DWORD objnum, FX_DWORD gennum, PARSE_CONTEXT* pContext, FX_BOOL bDecrypt)
 {
-    if (level > _PARSER_OBJECT_LEVLE_) {
+    CFX_AutoRestorer<int> restorer(&s_CurrentRecursionDepth);
+    if (++s_CurrentRecursionDepth > kParserMaxRecursionDepth) {
         return NULL;
     }
     FX_FILESIZE SavedPos = m_Pos;
@@ -2136,7 +2141,7 @@ CPDF_Object* CPDF_SyntaxParser::GetObject(CPDF_IndirectObjects* pObjList, FX_DWO
         }
         CPDF_Array* pArray = CPDF_Array::Create();
         while (1) {
-            CPDF_Object* pObj = GetObject(pObjList, objnum, gennum, level + 1);
+            CPDF_Object* pObj = GetObject(pObjList, objnum, gennum);
             if (pObj == NULL) {
                 return pArray;
             }
@@ -2188,7 +2193,7 @@ CPDF_Object* CPDF_SyntaxParser::GetObject(CPDF_IndirectObjects* pObjList, FX_DWO
             if (key == FX_BSTRC("/Contents")) {
                 dwSignValuePos = m_Pos;
             }
-            CPDF_Object* pObj = GetObject(pObjList, objnum, gennum, level + 1);
+            CPDF_Object* pObj = GetObject(pObjList, objnum, gennum);
             if (pObj == NULL) {
                 continue;
             }
@@ -2203,7 +2208,7 @@ CPDF_Object* CPDF_SyntaxParser::GetObject(CPDF_IndirectObjects* pObjList, FX_DWO
         if (IsSignatureDict(pDict)) {
             FX_FILESIZE dwSavePos = m_Pos;
             m_Pos = dwSignValuePos;
-            CPDF_Object* pObj = GetObject(pObjList, objnum, gennum, level + 1, NULL, FALSE);
+            CPDF_Object* pObj = GetObject(pObjList, objnum, gennum, NULL, FALSE);
             pDict->SetAt(FX_BSTRC("Contents"), pObj);
             m_Pos = dwSavePos;
         }
@@ -2238,10 +2243,10 @@ CPDF_Object* CPDF_SyntaxParser::GetObject(CPDF_IndirectObjects* pObjList, FX_DWO
     }
     return NULL;
 }
-CPDF_Object* CPDF_SyntaxParser::GetObjectByStrict(CPDF_IndirectObjects* pObjList, FX_DWORD objnum, FX_DWORD gennum,
-        FX_INT32 level, struct PARSE_CONTEXT* pContext)
+CPDF_Object* CPDF_SyntaxParser::GetObjectByStrict(CPDF_IndirectObjects* pObjList, FX_DWORD objnum, FX_DWORD gennum, struct PARSE_CONTEXT* pContext)
 {
-    if (level > _PARSER_OBJECT_LEVLE_) {
+    CFX_AutoRestorer<int> restorer(&s_CurrentRecursionDepth);
+    if (++s_CurrentRecursionDepth > kParserMaxRecursionDepth) {
         return NULL;
     }
     FX_FILESIZE SavedPos = m_Pos;
@@ -2318,7 +2323,7 @@ CPDF_Object* CPDF_SyntaxParser::GetObjectByStrict(CPDF_IndirectObjects* pObjList
         }
         CPDF_Array* pArray = CPDF_Array::Create();
         while (1) {
-            CPDF_Object* pObj = GetObject(pObjList, objnum, gennum, level + 1);
+            CPDF_Object* pObj = GetObject(pObjList, objnum, gennum);
             if (pObj == NULL) {
                 if (m_WordBuffer[0] == ']') {
                     return pArray;
@@ -2364,7 +2369,7 @@ CPDF_Object* CPDF_SyntaxParser::GetObjectByStrict(CPDF_IndirectObjects* pObjList
                 continue;
             }
             key = PDF_NameDecode(key);
-            CPDF_Object* pObj = GetObject(pObjList, objnum, gennum, level + 1);
+            CPDF_Object* pObj = GetObject(pObjList, objnum, gennum);
             if (pObj == NULL) {
                 if (pDict)
                     pDict->Release();
index ee32727..14a6532 100644 (file)
@@ -177,3 +177,8 @@ TEST_F(FPDFViewEmbeddertest, NamedDestsByName) {
   dest = FPDF_GetNamedDestByName(document(), "Bogus");
   EXPECT_EQ(nullptr, dest);
 }
+
+// The following tests pass if the document opens without crashing.
+TEST_F(FPDFViewEmbeddertest, Crashers) {
+  EXPECT_TRUE(OpenDocument("testing/resources/bug_451830.pdf"));
+}
diff --git a/testing/resources/bug_451830.pdf b/testing/resources/bug_451830.pdf
new file mode 100644 (file)
index 0000000..f209bb3
--- /dev/null
@@ -0,0 +1,14 @@
+%PDF-1.2
+%âãÏÓ
+7 0 obj <<
+  /Type /Font
+trailer
+<<///
+endobj
+4 0 obj <<
+  /Resources <<
+  /FT 7 0 R
+>>
+endstream
+endobj
+%%EOF