Set a recursion limit on CPDF_DataAvail::CheckPageNode
authorOliver Chang <ochang@chromium.org>
Thu, 22 Oct 2015 23:32:14 +0000 (16:32 -0700)
committerOliver Chang <ochang@chromium.org>
Thu, 22 Oct 2015 23:32:14 +0000 (16:32 -0700)
This limit mirrors FX_MAX_PAGE_LEVEL in fpdf_parser_document.cpp

R=thestig@chromium.org, tsepez@chromium.org

BUG=544880

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

core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_embeddertest.cpp
testing/resources/bug_544880.in [new file with mode: 0644]
testing/resources/bug_544880.pdf [new file with mode: 0644]

index 12e2a39..dd416fb 100644 (file)
@@ -2754,6 +2754,7 @@ class CPDF_DataAvail final : public IPDF_DataAvail {
  protected:
   static const int kMaxDataAvailRecursionDepth = 64;
   static int s_CurrentDataAvailRecursionDepth;
+  static const int kMaxPageRecursionDepth = 1024;
 
   FX_DWORD GetObjectSize(FX_DWORD objnum, FX_FILESIZE& offset);
   FX_BOOL IsObjectsAvail(CFX_PtrArray& obj_array,
@@ -2806,7 +2807,8 @@ class CPDF_DataAvail final : public IPDF_DataAvail {
   FX_BOOL CheckPageNode(CPDF_PageNode& pageNodes,
                         int32_t iPage,
                         int32_t& iCount,
-                        IFX_DownloadHints* pHints);
+                        IFX_DownloadHints* pHints,
+                        int level);
   FX_BOOL CheckUnkownPageNode(FX_DWORD dwPageNo,
                               CPDF_PageNode* pPageNode,
                               IFX_DownloadHints* pHints);
@@ -4193,7 +4195,11 @@ FX_BOOL CPDF_DataAvail::CheckUnkownPageNode(FX_DWORD dwPageNo,
 FX_BOOL CPDF_DataAvail::CheckPageNode(CPDF_PageNode& pageNodes,
                                       int32_t iPage,
                                       int32_t& iCount,
-                                      IFX_DownloadHints* pHints) {
+                                      IFX_DownloadHints* pHints,
+                                      int level) {
+  if (level >= kMaxPageRecursionDepth) {
+    return FALSE;
+  }
   int32_t iSize = pageNodes.m_childNode.GetSize();
   if (iSize <= 0 || iPage >= iSize) {
     m_docStatus = PDF_DATAAVAIL_ERROR;
@@ -4218,7 +4224,7 @@ FX_BOOL CPDF_DataAvail::CheckPageNode(CPDF_PageNode& pageNodes,
         }
         break;
       case PDF_PAGENODE_PAGES:
-        if (!CheckPageNode(*pNode, iPage, iCount, pHints)) {
+        if (!CheckPageNode(*pNode, iPage, iCount, pHints, level + 1)) {
           return FALSE;
         }
         break;
@@ -4251,7 +4257,7 @@ FX_BOOL CPDF_DataAvail::LoadDocPage(int32_t iPage, IFX_DownloadHints* pHints) {
     return TRUE;
   }
   int32_t iCount = -1;
-  return CheckPageNode(m_pageNodes, iPage, iCount, pHints);
+  return CheckPageNode(m_pageNodes, iPage, iCount, pHints, 0);
 }
 FX_BOOL CPDF_DataAvail::CheckPageCount(IFX_DownloadHints* pHints) {
   FX_BOOL bExist = FALSE;
index 96ea766..b6cfc4e 100644 (file)
@@ -19,3 +19,12 @@ TEST_F(FPDFParserEmbeddertest, Bug_481363) {
   EXPECT_NE(nullptr, page);
   UnloadPage(page);
 }
+
+TEST_F(FPDFParserEmbeddertest, Bug_544880) {
+  // Test self referencing /Pages object.
+  EXPECT_TRUE(OpenDocument("testing/resources/bug_544880.pdf"));
+  // Shouldn't crash. We don't check the return value here because we get the
+  // the count from the "/Count 1" in the testcase (at the time of writing)
+  // rather than the actual count (0).
+  (void)GetPageCount();
+}
diff --git a/testing/resources/bug_544880.in b/testing/resources/bug_544880.in
new file mode 100644 (file)
index 0000000..926a491
--- /dev/null
@@ -0,0 +1,19 @@
+{{header}}
+{{object 1 0}} <<
+  /Type /Catalog
+  /Pages 2 0 R
+>>
+endobj
+{{object 2 0}} <<
+  /Type /Pages
+  /Count 1
+  /Kids [2 0 R]
+>>
+endobj
+{{xref}}
+trailer <<
+  /Size 3
+  /Root 1 0 R
+>>
+{{startxref}}
+%%EOF
diff --git a/testing/resources/bug_544880.pdf b/testing/resources/bug_544880.pdf
new file mode 100644 (file)
index 0000000..4a8e5b9
--- /dev/null
@@ -0,0 +1,25 @@
+%PDF-1.7
+% ò¤ô
+1 0 obj <<
+  /Type /Catalog
+  /Pages 2 0 R
+>>
+endobj
+2 0 obj <<
+  /Type /Pages
+  /Count 1
+  /Kids [2 0 R]
+>>
+endobj
+xref
+0 3
+0000000000 65535 f 
+0000000015 00000 n 
+0000000068 00000 n 
+trailer <<
+  /Size 3
+  /Root 1 0 R
+>>
+startxref
+131
+%%EOF