From aab8f462ea3368ed4825172231131e1a10db978d Mon Sep 17 00:00:00 2001 From: Wei Li Date: Tue, 5 Jan 2016 12:34:00 -0800 Subject: Exit infinite loops for cross reference loading When cross reference sections form a loop, our code will enter an infinite loop. Add detection and exit code for v4 cross reference loading. V5 loading was done previously. R=thestig@chromium.org Review URL: https://codereview.chromium.org/1558093002 . --- .../src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp | 42 +++++++++++--------- fpdfsdk/src/fpdfview_embeddertest.cpp | 8 ++++ testing/resources/bug_xrefv4_loop.pdf | 45 ++++++++++++++++++++++ 3 files changed, 76 insertions(+), 19 deletions(-) create mode 100644 testing/resources/bug_xrefv4_loop.pdf diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp index c6e48e74a2..026247c3aa 100644 --- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp +++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp @@ -374,30 +374,25 @@ FX_BOOL CPDF_Parser::LoadAllCrossRefV4(FX_FILESIZE xrefpos) { CFX_FileSizeArray CrossRefList, XRefStreamList; CrossRefList.Add(xrefpos); XRefStreamList.Add(GetDirectInteger(m_pTrailer, "XRefStm")); - if (!CheckDirectType(m_pTrailer, "Prev", PDFOBJ_NUMBER)) { - return FALSE; - } - FX_FILESIZE newxrefpos = GetDirectInteger(m_pTrailer, "Prev"); - if (newxrefpos == xrefpos) { - return FALSE; - } - xrefpos = newxrefpos; + + std::set seen_xrefpos; + seen_xrefpos.insert(xrefpos); + // When |m_pTrailer| doesn't have Prev entry or Prev entry value is not + // numerical, GetDirectInteger() returns 0. Loading will end. + xrefpos = GetDirectInteger(m_pTrailer, "Prev"); while (xrefpos) { + // Check for circular references. + if (pdfium::ContainsKey(seen_xrefpos, xrefpos)) + return FALSE; + seen_xrefpos.insert(xrefpos); CrossRefList.InsertAt(0, xrefpos); LoadCrossRefV4(xrefpos, 0, TRUE); std::unique_ptr> pDict( LoadTrailerV4()); if (!pDict) return FALSE; + xrefpos = GetDirectInteger(pDict.get(), "Prev"); - if (!CheckDirectType(pDict.get(), "Prev", PDFOBJ_NUMBER)) - return FALSE; - - newxrefpos = GetDirectInteger(pDict.get(), "Prev"); - if (newxrefpos == xrefpos) - return FALSE; - - xrefpos = newxrefpos; XRefStreamList.InsertAt(0, pDict->GetInteger("XRefStm")); m_Trailers.Add(pDict.release()); } @@ -423,17 +418,26 @@ FX_BOOL CPDF_Parser::LoadLinearizedAllCrossRefV4(FX_FILESIZE xrefpos, CFX_FileSizeArray CrossRefList, XRefStreamList; CrossRefList.Add(xrefpos); XRefStreamList.Add(GetDirectInteger(m_pTrailer, "XRefStm")); + + std::set seen_xrefpos; + seen_xrefpos.insert(xrefpos); xrefpos = GetDirectInteger(m_pTrailer, "Prev"); while (xrefpos) { + // Check for circular references. + if (pdfium::ContainsKey(seen_xrefpos, xrefpos)) + return FALSE; + seen_xrefpos.insert(xrefpos); CrossRefList.InsertAt(0, xrefpos); LoadCrossRefV4(xrefpos, 0, TRUE); - CPDF_Dictionary* pDict = LoadTrailerV4(); + std::unique_ptr> pDict( + LoadTrailerV4()); if (!pDict) { return FALSE; } - xrefpos = GetDirectInteger(pDict, "Prev"); + xrefpos = GetDirectInteger(pDict.get(), "Prev"); + XRefStreamList.InsertAt(0, pDict->GetInteger("XRefStm")); - m_Trailers.Add(pDict); + m_Trailers.Add(pDict.release()); } for (int32_t i = 1; i < CrossRefList.GetSize(); i++) if (!LoadCrossRefV4(CrossRefList[i], XRefStreamList[i], FALSE)) { diff --git a/fpdfsdk/src/fpdfview_embeddertest.cpp b/fpdfsdk/src/fpdfview_embeddertest.cpp index 3be96e7aef..0f4666836f 100644 --- a/fpdfsdk/src/fpdfview_embeddertest.cpp +++ b/fpdfsdk/src/fpdfview_embeddertest.cpp @@ -204,3 +204,11 @@ TEST_F(FPDFViewEmbeddertest, Crasher_454695) { TEST_F(FPDFViewEmbeddertest, Hang_298) { EXPECT_FALSE(OpenDocument("bug_298.pdf")); } + +// Test if the document opens without infinite looping. +// Previously this test will hang in a loop inside LoadAllCrossRefV4. After +// the fix, LoadAllCrossRefV4 will return false after detecting a cross +// reference loop. Cross references will be rebuilt successfully. +TEST_F(FPDFViewEmbeddertest, CrossRefV4Loop) { + EXPECT_TRUE(OpenDocument("bug_xrefv4_loop.pdf")); +} diff --git a/testing/resources/bug_xrefv4_loop.pdf b/testing/resources/bug_xrefv4_loop.pdf new file mode 100644 index 0000000000..c1169773f7 --- /dev/null +++ b/testing/resources/bug_xrefv4_loop.pdf @@ -0,0 +1,45 @@ +%PDF-1.7 +% ò¤ô +1 0 obj << + /Type /Catalog + /Pages 2 0 R +>> +endobj +2 0 obj << + /Type /Pages + /Count 1 + /Kids [ + 3 0 R + ] +>> +endobj +3 0 obj << + /Type /Page + /Parent 2 0 R + /MediaBox [0 0 612 792] +>> +endobj +xref +0 2 +0000000000 65535 f +0000000015 00000 n +trailer << + /Prev 346 +>> +xref +2 1 +0000000068 00000 n +trailer << + /Prev 216 +>> +xref +3 1 +0000000139 00000 n +trailer << + /Size 4 + /Prev 291 + /Root 1 0 R +>> +startxref +346 +%%EOF -- cgit v1.2.3