Prevent infinite looping in CPDF_Parser::LoadAllCrossRefV5().
BUG=pdfium:298
R=weili@chromium.org
Review URL: https://codereview.chromium.org/1496703005 .
diff --git a/core/include/fpdfapi/fpdf_parser.h b/core/include/fpdfapi/fpdf_parser.h
index ec005de..dab0719 100644
--- a/core/include/fpdfapi/fpdf_parser.h
+++ b/core/include/fpdfapi/fpdf_parser.h
@@ -482,7 +482,7 @@
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 f64ba0d..bc5d3ed 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 @@
}
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<FX_FILESIZE> 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 @@
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 @@
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_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 @@
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<FX_FILESIZE> 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 35da9b6..3be96e7 100644
--- a/fpdfsdk/src/fpdfview_embeddertest.cpp
+++ b/fpdfsdk/src/fpdfview_embeddertest.cpp
@@ -199,3 +199,8 @@
// 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 0000000..b066778
--- /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 0000000..5b19559
--- /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
