From 7d1578a9ac083bf186dc3297885bfc56fdb78540 Mon Sep 17 00:00:00 2001 From: Wei Li Date: Tue, 1 Dec 2015 17:45:16 -0800 Subject: Merge to XFA: 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 TBR=jun_fang@foxitsoftware.com Review URL: https://codereview.chromium.org/1476163002 . (cherry picked from commit f14da1d58e8e12633c7a47e6efd5ffe43bb37b4b) Review URL: https://codereview.chromium.org/1486383002 . --- .../src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp | 18 +++----- .../fpdf_parser/fpdf_parser_parser_unittest.cpp | 11 +++++ .../parser_rebuildxref_error_notrailer.pdf | 51 ++++++++++++++++++++++ 3 files changed, 68 insertions(+), 12 deletions(-) create mode 100644 testing/resources/parser_rebuildxref_error_notrailer.pdf 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 -- cgit v1.2.3