From e96e6fdddaffa2b4b82df4d4d551333939fb78c9 Mon Sep 17 00:00:00 2001
From: Tom Sepez <tsepez@chromium.org>
Date: Tue, 3 Apr 2018 15:02:37 +0000
Subject: Off-by-one in CPDF_StreamParser::ParseNextElement()

Limit the token to 255 bytes + NUL.  Also, shuffle fields in
cpdf_streamparser to allow memory tools to better check this
inline array.

Bug: 828049
Change-Id: I444f2b4c6958167577d9cd76c06805baf7d5c26c
Reviewed-on: https://pdfium-review.googlesource.com/29530
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: dsinclair <dsinclair@chromium.org>
---
 core/fpdfapi/page/cpdf_streamparser.cpp          | 14 ++++++--------
 core/fpdfapi/page/cpdf_streamparser.h            |  9 +++++----
 core/fpdfapi/parser/cpdf_parser_embeddertest.cpp |  7 +++++++
 testing/resources/bug_828049.pdf                 |  6 ++++++
 4 files changed, 24 insertions(+), 12 deletions(-)
 create mode 100644 testing/resources/bug_828049.pdf

diff --git a/core/fpdfapi/page/cpdf_streamparser.cpp b/core/fpdfapi/page/cpdf_streamparser.cpp
index 71c8c8d29a..3b6d12038f 100644
--- a/core/fpdfapi/page/cpdf_streamparser.cpp
+++ b/core/fpdfapi/page/cpdf_streamparser.cpp
@@ -33,7 +33,6 @@
 namespace {
 
 const uint32_t kMaxNestedParsingLevel = 512;
-const uint32_t kMaxWordBuffer = 256;
 const size_t kMaxStringLength = 32767;
 
 uint32_t DecodeAllScanlines(std::unique_ptr<CCodec_ScanlineDecoder> pDecoder,
@@ -103,12 +102,12 @@ uint32_t DecodeInlineStream(const uint8_t* src_buf,
 }  // namespace
 
 CPDF_StreamParser::CPDF_StreamParser(const uint8_t* pData, uint32_t dwSize)
-    : m_pBuf(pData), m_Size(dwSize), m_Pos(0), m_pPool(nullptr) {}
+    : m_Size(dwSize), m_Pos(0), m_WordSize(0), m_pBuf(pData) {}
 
 CPDF_StreamParser::CPDF_StreamParser(const uint8_t* pData,
                                      uint32_t dwSize,
                                      const WeakPtr<ByteStringPool>& pPool)
-    : m_pBuf(pData), m_Size(dwSize), m_Pos(0), m_pPool(pPool) {}
+    : m_Size(dwSize), m_Pos(0), m_WordSize(0), m_pBuf(pData), m_pPool(pPool) {}
 
 CPDF_StreamParser::~CPDF_StreamParser() {}
 
@@ -257,7 +256,7 @@ CPDF_StreamParser::SyntaxType CPDF_StreamParser::ParseNextElement() {
 
   bool bIsNumber = true;
   while (1) {
-    if (m_WordSize < kMaxWordBuffer)
+    if (m_WordSize < kMaxWordLength)
       m_WordBuffer[m_WordSize++] = ch;
 
     if (!PDFCharIsNumeric(ch))
@@ -424,8 +423,7 @@ void CPDF_StreamParser::GetNextWord(bool& bIsNumber) {
           m_Pos--;
           return;
         }
-
-        if (m_WordSize < kMaxWordBuffer)
+        if (m_WordSize < kMaxWordLength)
           m_WordBuffer[m_WordSize++] = ch;
       }
     } else if (ch == '<') {
@@ -449,13 +447,13 @@ void CPDF_StreamParser::GetNextWord(bool& bIsNumber) {
   }
 
   while (1) {
-    if (m_WordSize < kMaxWordBuffer)
+    if (m_WordSize < kMaxWordLength)
       m_WordBuffer[m_WordSize++] = ch;
     if (!PDFCharIsNumeric(ch))
       bIsNumber = false;
-
     if (!PositionIsInBounds())
       return;
+
     ch = m_pBuf[m_Pos++];
     if (PDFCharIsDelimiter(ch) || PDFCharIsWhitespace(ch)) {
       m_Pos--;
diff --git a/core/fpdfapi/page/cpdf_streamparser.h b/core/fpdfapi/page/cpdf_streamparser.h
index 158726b026..bdd07643ce 100644
--- a/core/fpdfapi/page/cpdf_streamparser.h
+++ b/core/fpdfapi/page/cpdf_streamparser.h
@@ -44,19 +44,20 @@ class CPDF_StreamParser {
 
  private:
   friend class cpdf_streamparser_ReadHexString_Test;
+  static const uint32_t kMaxWordLength = 255;
 
   void GetNextWord(bool& bIsNumber);
   ByteString ReadString();
   ByteString ReadHexString();
   bool PositionIsInBounds() const;
 
+  uint32_t m_Size;      // Length in bytes of m_pBuf.
+  uint32_t m_Pos;       // Current byte position within m_pBuf.
+  uint32_t m_WordSize;  // Current byte position within m_WordBuffer.
   const uint8_t* m_pBuf;
-  uint32_t m_Size;  // Length in bytes of m_pBuf.
-  uint32_t m_Pos;   // Current byte position within m_pBuf.
-  uint8_t m_WordBuffer[256];
-  uint32_t m_WordSize;
   std::unique_ptr<CPDF_Object> m_pLastObj;
   WeakPtr<ByteStringPool> m_pPool;
+  uint8_t m_WordBuffer[kMaxWordLength + 1];  // Include space for NUL.
 };
 
 #endif  // CORE_FPDFAPI_PAGE_CPDF_STREAMPARSER_H_
diff --git a/core/fpdfapi/parser/cpdf_parser_embeddertest.cpp b/core/fpdfapi/parser/cpdf_parser_embeddertest.cpp
index 4109715a9c..3b8f550253 100644
--- a/core/fpdfapi/parser/cpdf_parser_embeddertest.cpp
+++ b/core/fpdfapi/parser/cpdf_parser_embeddertest.cpp
@@ -73,3 +73,10 @@ TEST_F(CPDFParserEmbeddertest, LoadMainCrossRefTable) {
   FPDFText_ClosePage(text_page);
   UnloadPage(page);
 }
+
+TEST_F(CPDFParserEmbeddertest, Bug_828049) {
+  EXPECT_TRUE(OpenDocument("bug_828049.pdf"));
+  FPDF_PAGE page = LoadPage(0);
+  EXPECT_NE(nullptr, page);
+  UnloadPage(page);
+}
diff --git a/testing/resources/bug_828049.pdf b/testing/resources/bug_828049.pdf
new file mode 100644
index 0000000000..d8942bb674
--- /dev/null
+++ b/testing/resources/bug_828049.pdf
@@ -0,0 +1,6 @@
+%PDF
+1 0 obj<</Pages<</Contents <<
+>>stream
+0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+endobj
+trailer<</Root 1 0 R>>
\ No newline at end of file
-- 
cgit v1.2.3