commit 6438c4f36da162f72e0d53e8fff45cd6687b7f5c
author Nicolas Pena <npm@chromium.org> Fri Jan 27 10:05:36 2017 -0500
committer Chromium commit bot <commit-bot@chromium.org> Fri Jan 27 15:32:35 2017 +0000
tree 0efaccaea0b15375432c387ebc8ee5ce1126254e
parent d532036fbb0efa4687f89598ff37518e3825c7b9

Limit parsing recursion levels in CPDF_StreamParser

We currently only limit the array recursion levels. This recursion
level may also be reset when parsing. This is insufficient to protect
against stack overflows.

BUG=681920

Change-Id: I69bd0c912fb45c0e68b9b9fa961d43f0adc9bdd3
Reviewed-on: https://pdfium-review.googlesource.com/2434
Commit-Queue: Nicolás Peña <npm@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>

core/fpdfapi/page/cpdf_streamcontentparser.cpp

diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
index 2426027..0e78612 100644
--- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp
+++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
@@ -628,7 +628,7 @@
       break;
     }
     CFX_ByteString key(m_pSyntax->GetWord().Mid(1));
-    auto pObj = m_pSyntax->ReadNextObject(false, 0);
+    auto pObj = m_pSyntax->ReadNextObject(false, false, 0);
     if (!key.IsEmpty()) {
       uint32_t dwObjNum = pObj ? pObj->GetObjNum() : 0;
       if (dwObjNum)

core/fpdfapi/page/cpdf_streamparser.cpp

diff --git a/core/fpdfapi/page/cpdf_streamparser.cpp b/core/fpdfapi/page/cpdf_streamparser.cpp
index 14efba2..294d72c 100644
--- a/core/fpdfapi/page/cpdf_streamparser.cpp
+++ b/core/fpdfapi/page/cpdf_streamparser.cpp
@@ -29,7 +29,7 @@
 
 namespace {
 
-const uint32_t kMaxNestedArrayLevel = 512;
+const uint32_t kMaxNestedParsingLevel = 512;
 const uint32_t kMaxWordBuffer = 256;
 const FX_STRSIZE kMaxStringLength = 32767;
 
@@ -255,7 +255,7 @@
 
   if (PDFCharIsDelimiter(ch) && ch != '/') {
     m_Pos--;
-    m_pLastObj = ReadNextObject(false, 0);
+    m_pLastObj = ReadNextObject(false, false, 0);
     return Others;
   }
 
@@ -305,10 +305,12 @@
 
 std::unique_ptr<CPDF_Object> CPDF_StreamParser::ReadNextObject(
     bool bAllowNestedArray,
-    uint32_t dwInArrayLevel) {
+    bool bInArray,
+    uint32_t dwRecursionLevel) {
   bool bIsNumber;
+  // Must get the next word before returning to avoid infinite loops.
   GetNextWord(bIsNumber);
-  if (!m_WordSize)
+  if (!m_WordSize || dwRecursionLevel > kMaxNestedParsingLevel)
     return nullptr;
 
   if (bIsNumber) {
@@ -344,7 +346,8 @@
 
       CFX_ByteString key =
           PDF_NameDecode(CFX_ByteStringC(m_WordBuffer + 1, m_WordSize - 1));
-      std::unique_ptr<CPDF_Object> pObj = ReadNextObject(true, 0);
+      std::unique_ptr<CPDF_Object> pObj =
+          ReadNextObject(true, bInArray, dwRecursionLevel + 1);
       if (!pObj)
         return nullptr;
 
@@ -355,15 +358,13 @@
   }
 
   if (first_char == '[') {
-    if ((!bAllowNestedArray && dwInArrayLevel) ||
-        dwInArrayLevel > kMaxNestedArrayLevel) {
+    if ((!bAllowNestedArray && bInArray))
       return nullptr;
-    }
 
     auto pArray = pdfium::MakeUnique<CPDF_Array>();
     while (1) {
       std::unique_ptr<CPDF_Object> pObj =
-          ReadNextObject(bAllowNestedArray, dwInArrayLevel + 1);
+          ReadNextObject(bAllowNestedArray, true, dwRecursionLevel + 1);
       if (pObj) {
         pArray->Add(std::move(pObj));
         continue;

core/fpdfapi/page/cpdf_streamparser.h

diff --git a/core/fpdfapi/page/cpdf_streamparser.h b/core/fpdfapi/page/cpdf_streamparser.h
index dc3b7dc..fdc418c 100644
--- a/core/fpdfapi/page/cpdf_streamparser.h
+++ b/core/fpdfapi/page/cpdf_streamparser.h
@@ -35,7 +35,8 @@
   void SetPos(uint32_t pos) { m_Pos = pos; }
   std::unique_ptr<CPDF_Object> GetObject() { return std::move(m_pLastObj); }
   std::unique_ptr<CPDF_Object> ReadNextObject(bool bAllowNestedArray,
-                                              uint32_t dwInArrayLevel);
+                                              bool bInArray,
+                                              uint32_t dwRecursionLevel);
   std::unique_ptr<CPDF_Stream> ReadInlineStream(
       CPDF_Document* pDoc,
       std::unique_ptr<CPDF_Dictionary> pDict,

testing/libfuzzer/pdf_streamparser_fuzzer.cc

diff --git a/testing/libfuzzer/pdf_streamparser_fuzzer.cc b/testing/libfuzzer/pdf_streamparser_fuzzer.cc
index 5cfa318..46113d4 100644
--- a/testing/libfuzzer/pdf_streamparser_fuzzer.cc
+++ b/testing/libfuzzer/pdf_streamparser_fuzzer.cc
@@ -10,7 +10,8 @@
 
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
   CPDF_StreamParser parser(data, size);
-  while (std::unique_ptr<CPDF_Object> pObj = parser.ReadNextObject(true, 0))
+  while (std::unique_ptr<CPDF_Object> pObj =
+             parser.ReadNextObject(true, false, 0))
     continue;
 
   return 0;