Add basic checking for RebuildCrossRef

RebuildCrossRef function returns false when we can not find file trailer
or any indirect object. This serves as a basic file format checking.

BUG=pdfium:215
R=jun_fang@foxitsoftware.com

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

3 files changed, 68 insertions, 12 deletions

diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
index 2105635687..d5664c29ab 100644
--- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
+++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
@@ -247,12 +247,9 @@ FX_DWORD CPDF_Parser::StartParse(IFX_FileRead* pFileAccess,
   }
   FXSYS_qsort(m_SortedOffset.GetData(), m_SortedOffset.GetSize(),
               sizeof(FX_FILESIZE), CompareFileSize);
-  FX_DWORD RootObjNum = GetRootObjNum();
-  if (RootObjNum == 0) {
+  if (GetRootObjNum() == 0) {
     ReleaseEncryptHandler();
-    RebuildCrossRef();
-    RootObjNum = GetRootObjNum();
-    if (RootObjNum == 0)
+    if (!RebuildCrossRef() || GetRootObjNum() == 0)
       return PDFPARSE_ERROR_FORMAT;
 
     dwRet = SetEncryptHandler();
@@ -975,7 +972,7 @@ FX_BOOL CPDF_Parser::RebuildCrossRef() {
     m_SortedOffset.Add(offset);
   }
   FX_Free(buffer);
-  return TRUE;
+  return m_pTrailer && m_CrossRef.GetSize() > 0;
 }
 
 FX_BOOL CPDF_Parser::LoadCrossRefV5(FX_FILESIZE pos,
@@ -1608,14 +1605,11 @@ FX_DWORD CPDF_Parser::StartAsynParse(IFX_FileRead* pFileAccess,
   }
   FXSYS_qsort(m_SortedOffset.GetData(), m_SortedOffset.GetSize(),
               sizeof(FX_FILESIZE), CompareFileSize);
-  FX_DWORD RootObjNum = GetRootObjNum();
-  if (RootObjNum == 0) {
+  if (GetRootObjNum() == 0) {
     ReleaseEncryptHandler();
-    RebuildCrossRef();
-    RootObjNum = GetRootObjNum();
-    if (RootObjNum == 0) {
+    if (!RebuildCrossRef() || GetRootObjNum() == 0)
       return PDFPARSE_ERROR_FORMAT;
-    }
+
     dwRet = SetEncryptHandler();
     if (dwRet != PDFPARSE_ERROR_SUCCESS) {
       return dwRet;
diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_unittest.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_unittest.cpp
index 8e953a6ee3..640feac9aa 100644
--- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_unittest.cpp
+++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser_unittest.cpp
@@ -26,6 +26,7 @@ class CPDF_TestParser : public CPDF_Parser {
   // Add test case as private friend so that RebuildCrossRef in CPDF_Parser
   // can be accessed.
   FRIEND_TEST(fpdf_parser_parser, RebuildCrossRefCorrectly);
+  FRIEND_TEST(fpdf_parser_parser, RebuildCrossRefFailed);
 };
 
 // TODO(thestig) Using unique_ptr with ReleaseDeleter is still not ideal.
@@ -211,3 +212,13 @@ TEST(fpdf_parser_parser, RebuildCrossRefCorrectly) {
     EXPECT_EQ(versions[i], parser.m_ObjVersion.GetAt(i));
   }
 }
+
+TEST(fpdf_parser_parser, RebuildCrossRefFailed) {
+  CPDF_TestParser parser;
+  std::string test_file;
+  ASSERT_TRUE(PathService::GetTestFilePath(
+      "parser_rebuildxref_error_notrailer.pdf", &test_file));
+  ASSERT_TRUE(parser.InitTest(test_file.c_str())) << test_file;
+
+  ASSERT_FALSE(parser.RebuildCrossRef());
+}
\ No newline at end of file
diff --git a/testing/resources/parser_rebuildxref_error_notrailer.pdf b/testing/resources/parser_rebuildxref_error_notrailer.pdf
new file mode 100644
index 0000000000..93f56fba9b
--- /dev/null
+++ b/testing/resources/parser_rebuildxref_error_notrailer.pdf
@@ -0,0 +1,51 @@
+%PDF-1.7
+% ò¤ô
+1 0 obj <<
+  /Type /Catalog
+  /Pages 2 2 R
+>>
+2 2 obj <<
+  /Type /Pages
+  /MediaBox [ 0 0 200 200 ]
+  /Count 1
+  /Kids [ 3 4 R ]
+>>
+endobj
+3 4 obj <<
+  /Type /Page
+  /Parent 2 2 R
+  /Resources <<
+    /Font <<
+      /F1 4 6 R
+      /F2 5 8 R
+    >>
+  >>
+  /Contents 6 0 R
+>>
+endobj
+4 6 obj <<
+  /Type /Font
+  /Subtype /Type1
+  /BaseFont /Times-Roman
+>>
+endobj
+5 8 obj <<
+  /Type /Font
+  /Subtype /Type1
+  /BaseFont /Helvetica
+>>
+endobj
+6 0 obj <<
+>>
+stream
+BT
+20 50 Td
+/F1 12 Tf
+(Hello, world!) Tj
+0 50 Td
+/F2 16 Tf
+(Goodbye, world!) Tj
+ET
+endstream
+endobj
+%%EOF