From 6438c4f36da162f72e0d53e8fff45cd6687b7f5c Mon Sep 17 00:00:00 2001
From: Nicolas Pena <npm@chromium.org>
Date: Fri, 27 Jan 2017 10:05:36 -0500
Subject: Limit parsing recursion levels in CPDF_StreamParser
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 |  2 +-
 core/fpdfapi/page/cpdf_streamparser.cpp        | 19 ++++++++++---------
 core/fpdfapi/page/cpdf_streamparser.h          |  3 ++-
 testing/libfuzzer/pdf_streamparser_fuzzer.cc   |  3 ++-
 4 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
index 2426027976..0e78612bc9 100644
--- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp
+++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp
@@ -628,7 +628,7 @@ void CPDF_StreamContentParser::Handle_BeginImage() {
       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)
diff --git a/core/fpdfapi/page/cpdf_streamparser.cpp b/core/fpdfapi/page/cpdf_streamparser.cpp
index 14efba2200..294d72c950 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 @@ CPDF_StreamParser::SyntaxType CPDF_StreamParser::ParseNextElement() {
 
   if (PDFCharIsDelimiter(ch) && ch != '/') {
     m_Pos--;
-    m_pLastObj = ReadNextObject(false, 0);
+    m_pLastObj = ReadNextObject(false, false, 0);
     return Others;
   }
 
@@ -305,10 +305,12 @@ CPDF_StreamParser::SyntaxType CPDF_StreamParser::ParseNextElement() {
 
 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 @@ std::unique_ptr<CPDF_Object> CPDF_StreamParser::ReadNextObject(
 
       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 @@ std::unique_ptr<CPDF_Object> CPDF_StreamParser::ReadNextObject(
   }
 
   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;
diff --git a/core/fpdfapi/page/cpdf_streamparser.h b/core/fpdfapi/page/cpdf_streamparser.h
index dc3b7dcb40..fdc418c634 100644
--- a/core/fpdfapi/page/cpdf_streamparser.h
+++ b/core/fpdfapi/page/cpdf_streamparser.h
@@ -35,7 +35,8 @@ class CPDF_StreamParser {
   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,
diff --git a/testing/libfuzzer/pdf_streamparser_fuzzer.cc b/testing/libfuzzer/pdf_streamparser_fuzzer.cc
index 5cfa318c60..46113d42c6 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;
-- 
cgit v1.2.3