From 93181f9a20db7ac706bb9405750303db93762a5b Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Wed, 9 Dec 2015 20:15:18 -0800 Subject: Prevent infinite looping in CPDF_Parser::LoadAllCrossRefV5(). BUG=pdfium:298 R=weili@chromium.org Review URL: https://codereview.chromium.org/1496703005 . --- core/include/fpdfapi/fpdf_parser.h | 2 +- .../src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp | 39 ++++++++++++++-------- fpdfsdk/src/fpdfview_embeddertest.cpp | 5 +++ testing/resources/bug_298.in | 21 ++++++++++++ testing/resources/bug_298.pdf | 22 ++++++++++++ 5 files changed, 75 insertions(+), 14 deletions(-) create mode 100644 testing/resources/bug_298.in create mode 100644 testing/resources/bug_298.pdf diff --git a/core/include/fpdfapi/fpdf_parser.h b/core/include/fpdfapi/fpdf_parser.h index ec005de812..dab0719f26 100644 --- a/core/include/fpdfapi/fpdf_parser.h +++ b/core/include/fpdfapi/fpdf_parser.h @@ -482,7 +482,7 @@ class CPDF_Parser { FX_BOOL bSkip, FX_BOOL bFirst); - FX_BOOL LoadCrossRefV5(FX_FILESIZE pos, FX_FILESIZE& prev, FX_BOOL bMainXRef); + FX_BOOL LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef); CPDF_Dictionary* LoadTrailerV4(); diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp index f64ba0deb0..bc5d3edc2b 100644 --- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp +++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp @@ -582,21 +582,29 @@ bool CPDF_Parser::LoadCrossRefV4(FX_FILESIZE pos, } m_Syntax.RestorePos(SavedPos + count * recordsize); } - return !streampos || LoadCrossRefV5(streampos, streampos, FALSE); + return !streampos || LoadCrossRefV5(&streampos, FALSE); } FX_BOOL CPDF_Parser::LoadAllCrossRefV5(FX_FILESIZE xrefpos) { - if (!LoadCrossRefV5(xrefpos, xrefpos, TRUE)) { + if (!LoadCrossRefV5(&xrefpos, TRUE)) { return FALSE; } - while (xrefpos) - if (!LoadCrossRefV5(xrefpos, xrefpos, FALSE)) { + std::set seen_xrefpos; + while (xrefpos) { + seen_xrefpos.insert(xrefpos); + if (!LoadCrossRefV5(&xrefpos, FALSE)) { + return FALSE; + } + // Check for circular references. + if (seen_xrefpos.find(xrefpos) != seen_xrefpos.end()) { return FALSE; } + } m_ObjectStreamMap.InitHashTable(101, FALSE); m_bXRefStream = TRUE; return TRUE; } + FX_BOOL CPDF_Parser::RebuildCrossRef() { m_CrossRef.RemoveAll(); m_V5Type.RemoveAll(); @@ -975,10 +983,8 @@ FX_BOOL CPDF_Parser::RebuildCrossRef() { return m_pTrailer && m_CrossRef.GetSize() > 0; } -FX_BOOL CPDF_Parser::LoadCrossRefV5(FX_FILESIZE pos, - FX_FILESIZE& prev, - FX_BOOL bMainXRef) { - CPDF_Object* pObject = ParseIndirectObjectAt(m_pDocument, pos, 0, nullptr); +FX_BOOL CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef) { + CPDF_Object* pObject = ParseIndirectObjectAt(m_pDocument, *pos, 0, nullptr); if (!pObject) return FALSE; @@ -997,7 +1003,7 @@ FX_BOOL CPDF_Parser::LoadCrossRefV5(FX_FILESIZE pos, if (!pStream) return FALSE; - prev = pStream->GetDict()->GetInteger(FX_BSTRC("Prev")); + *pos = pStream->GetDict()->GetInteger(FX_BSTRC("Prev")); int32_t size = pStream->GetDict()->GetInteger(FX_BSTRC("Size")); if (size < 0) { pStream->Release(); @@ -1563,7 +1569,7 @@ FX_DWORD CPDF_Parser::StartAsynParse(IFX_FileRead* pFileAccess, FX_BOOL bXRefRebuilt = FALSE; FX_BOOL bLoadV4 = FALSE; if (!(bLoadV4 = LoadCrossRefV4(dwFirstXRefOffset, 0, FALSE, FALSE)) && - !LoadCrossRefV5(dwFirstXRefOffset, dwFirstXRefOffset, TRUE)) { + !LoadCrossRefV5(&dwFirstXRefOffset, TRUE)) { if (!RebuildCrossRef()) { return PDFPARSE_ERROR_FORMAT; } @@ -1623,13 +1629,20 @@ FX_DWORD CPDF_Parser::StartAsynParse(IFX_FileRead* pFileAccess, return PDFPARSE_ERROR_SUCCESS; } FX_BOOL CPDF_Parser::LoadLinearizedAllCrossRefV5(FX_FILESIZE xrefpos) { - if (!LoadCrossRefV5(xrefpos, xrefpos, FALSE)) { + if (!LoadCrossRefV5(&xrefpos, FALSE)) { return FALSE; } - while (xrefpos) - if (!LoadCrossRefV5(xrefpos, xrefpos, FALSE)) { + std::set seen_xrefpos; + while (xrefpos) { + seen_xrefpos.insert(xrefpos); + if (!LoadCrossRefV5(&xrefpos, FALSE)) { + return FALSE; + } + // Check for circular references. + if (seen_xrefpos.find(xrefpos) != seen_xrefpos.end()) { return FALSE; } + } m_ObjectStreamMap.InitHashTable(101, FALSE); m_bXRefStream = TRUE; return TRUE; diff --git a/fpdfsdk/src/fpdfview_embeddertest.cpp b/fpdfsdk/src/fpdfview_embeddertest.cpp index 35da9b6caf..3be96e7aef 100644 --- a/fpdfsdk/src/fpdfview_embeddertest.cpp +++ b/fpdfsdk/src/fpdfview_embeddertest.cpp @@ -199,3 +199,8 @@ TEST_F(FPDFViewEmbeddertest, Crasher_454695) { // Document is damanged and can't be opened. EXPECT_FALSE(OpenDocument("bug_454695.pdf")); } + +// The following tests pass if the document opens without infinite looping. +TEST_F(FPDFViewEmbeddertest, Hang_298) { + EXPECT_FALSE(OpenDocument("bug_298.pdf")); +} diff --git a/testing/resources/bug_298.in b/testing/resources/bug_298.in new file mode 100644 index 0000000000..b066778a49 --- /dev/null +++ b/testing/resources/bug_298.in @@ -0,0 +1,21 @@ +{{header}} +{{object 1 0}} << +>> +endobj +{{object 2 0}} << + /Type /Catalog + /Length 0 + /Filter ASCIIHexDecode + /Prev 35 + /W [1 2 3] +>> +stream +endstream +endobj +trailer << + /Size 6 + /Root 1 0 R +>> +startxref +35 +%%EOF diff --git a/testing/resources/bug_298.pdf b/testing/resources/bug_298.pdf new file mode 100644 index 0000000000..5b195597fd --- /dev/null +++ b/testing/resources/bug_298.pdf @@ -0,0 +1,22 @@ +%PDF-1.7 +% ò¤ô +1 0 obj << +>> +endobj +2 0 obj << + /Type /Catalog + /Length 0 + /Filter ASCIIHexDecode + /Prev 35 + /W [1 2 3] +>> +stream +endstream +endobj +trailer << + /Size 6 + /Root 1 0 R +>> +startxref +35 +%%EOF -- cgit v1.2.3