Fix an infinite loop in CPDF_Parser::RebuildCrossRef().

BUG=pdfium:325
R=weili@chromium.org

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

4 files changed, 45 insertions, 16 deletions

diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
index 51c779fb86..4aa99d08e3 100644
--- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
+++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
@@ -6,6 +6,7 @@
 
 #include "parser_int.h"
 
+#include <algorithm>
 #include <memory>
 #include <set>
 #include <utility>
@@ -619,21 +620,24 @@ FX_BOOL CPDF_Parser::RebuildCrossRef() {
   }
   int32_t status = 0;
   int32_t inside_index = 0;
-  FX_DWORD objnum = 0, gennum = 0;
+  FX_DWORD objnum = 0;
+  FX_DWORD gennum = 0;
   int32_t depth = 0;
-  uint8_t* buffer = FX_Alloc(uint8_t, 4096);
+  const FX_DWORD kBufferSize = 4096;
+  std::vector<uint8_t> buffer(kBufferSize);
   FX_FILESIZE pos = m_Syntax.m_HeaderOffset;
-  FX_FILESIZE start_pos = 0, start_pos1 = 0;
-  FX_FILESIZE last_obj = -1, last_xref = -1, last_trailer = -1;
+  FX_FILESIZE start_pos = 0;
+  FX_FILESIZE start_pos1 = 0;
+  FX_FILESIZE last_obj = -1;
+  FX_FILESIZE last_xref = -1;
+  FX_FILESIZE last_trailer = -1;
   while (pos < m_Syntax.m_FileLen) {
-    FX_BOOL bOverFlow = FALSE;
-    FX_DWORD size = (FX_DWORD)(m_Syntax.m_FileLen - pos);
-    if (size > 4096) {
-      size = 4096;
-    }
-    if (!m_Syntax.m_pFileAccess->ReadBlock(buffer, pos, size)) {
+    const FX_FILESIZE saved_pos = pos;
+    bool bOverFlow = false;
+    FX_DWORD size = std::min((FX_DWORD)(m_Syntax.m_FileLen - pos), kBufferSize);
+    if (!m_Syntax.m_pFileAccess->ReadBlock(buffer.data(), pos, size))
       break;
-    }
+
     for (FX_DWORD i = 0; i < size; i++) {
       uint8_t byte = buffer[i];
       switch (status) {
@@ -807,7 +811,7 @@ FX_BOOL CPDF_Parser::RebuildCrossRef() {
                 FX_FILESIZE nLen = obj_end - obj_pos - offset;
                 if ((FX_DWORD)nLen > size - i) {
                   pos = obj_end + m_Syntax.m_HeaderOffset;
-                  bOverFlow = TRUE;
+                  bOverFlow = true;
                 } else {
                   i += (FX_DWORD)nLen;
                 }
@@ -854,11 +858,11 @@ FX_BOOL CPDF_Parser::RebuildCrossRef() {
                       if (!pRoot ||
                           (pRef && IsValidObjectNumber(pRef->GetRefObjNum()) &&
                            m_ObjectInfo[pRef->GetRefObjNum()].pos != 0)) {
-                        FX_POSITION pos = pTrailer->GetStartPos();
-                        while (pos) {
+                        FX_POSITION trailer_pos = pTrailer->GetStartPos();
+                        while (trailer_pos) {
                           CFX_ByteString key;
                           CPDF_Object* pElement =
-                              pTrailer->GetNextElement(pos, key);
+                              pTrailer->GetNextElement(trailer_pos, key);
                           FX_DWORD dwObjNum =
                               pElement ? pElement->GetObjNum() : 0;
                           if (dwObjNum) {
@@ -965,6 +969,11 @@ FX_BOOL CPDF_Parser::RebuildCrossRef() {
       }
     }
     pos += size;
+
+    // If the position has not changed at all in a loop iteration, then break
+    // out to prevent infinite looping.
+    if (pos == saved_pos)
+      break;
   }
   if (last_xref != -1 && last_xref > last_obj) {
     last_trailer = last_xref;
@@ -978,7 +987,6 @@ FX_BOOL CPDF_Parser::RebuildCrossRef() {
   if (!pResult) {
     m_SortedOffset.Add(offset);
   }
-  FX_Free(buffer);
   return m_pTrailer && !m_ObjectInfo.empty();
 }
 
diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_embeddertest.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_embeddertest.cpp
index 1929db7e8f..fba4117433 100644
--- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_embeddertest.cpp
+++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_embeddertest.cpp
@@ -33,3 +33,8 @@ TEST_F(FPDFParserEmbeddertest, Bug_544880) {
 TEST_F(FPDFParserEmbeddertest, Feature_Linearized_Loading) {
   EXPECT_TRUE(OpenDocument("feature_linearized_loading.pdf", true));
 }
+
+TEST_F(FPDFParserEmbeddertest, Bug_325) {
+  EXPECT_FALSE(OpenDocument("bug_325_a.pdf"));
+  EXPECT_FALSE(OpenDocument("bug_325_b.pdf"));
+}
diff --git a/testing/resources/bug_325_a.pdf b/testing/resources/bug_325_a.pdf
new file mode 100644
index 0000000000..1d0fae702b
--- /dev/null
+++ b/testing/resources/bug_325_a.pdf
@@ -0,0 +1,8 @@
+%PDF-.2
+xref
+0 3
+0000000000 655%ûPDF-1.5
+%âãÏÓ
+1 5f

+ 30080000009 0000 obj
+/<0<*ar
\ No newline at end of file
diff --git a/testing/resources/bug_325_b.pdf b/testing/resources/bug_325_b.pdf
new file mode 100644
index 0000000000..943a36831a
--- /dev/null
+++ b/testing/resources/bug_325_b.pdf
@@ -0,0 +1,8 @@
+%PDF-P%PDF-1.2
+Dxref
+0 3
+0000000000 655%PDF-1.5
+%âãÏÓ
+1 f5

+ 30080000009 0000 obj
+/<0<*ar
\ No newline at end of file