From e80685c12c3620c178a2286e00dd03c7886f8785 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Mon, 26 Jan 2015 16:59:09 -0800 Subject: Merge to XFA: Fix infinite recursion in CPDF_Parser::ParseIndirectObjectAt(). Orignal Review URL: https://codereview.chromium.org/875263002 TBR=thestig@chromium.org Review URL: https://codereview.chromium.org/880753002 --- core/include/fpdfapi/fpdf_parser.h | 7 +++-- core/include/fxcrt/fx_basic.h | 15 ++++++++++ .../src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp | 33 +++++++++++++--------- fpdfsdk/src/fpdfview_embeddertest.cpp | 5 ++++ testing/resources/bug_451830.pdf | 14 +++++++++ 5 files changed, 57 insertions(+), 17 deletions(-) create mode 100644 testing/resources/bug_451830.pdf diff --git a/core/include/fpdfapi/fpdf_parser.h b/core/include/fpdfapi/fpdf_parser.h index 3f9bda5ad8..e1a0f5cf1a 100644 --- a/core/include/fpdfapi/fpdf_parser.h +++ b/core/include/fpdfapi/fpdf_parser.h @@ -261,10 +261,10 @@ public: m_Pos = pos; } - CPDF_Object* GetObject(CPDF_IndirectObjects* pObjList, FX_DWORD objnum, FX_DWORD gennum, int level, struct PARSE_CONTEXT* pContext = NULL, FX_BOOL bDecrypt = TRUE); + CPDF_Object* GetObject(CPDF_IndirectObjects* pObjList, FX_DWORD objnum, FX_DWORD gennum, struct PARSE_CONTEXT* pContext = NULL, FX_BOOL bDecrypt = TRUE); - CPDF_Object* GetObjectByStrict(CPDF_IndirectObjects* pObjList, FX_DWORD objnum, FX_DWORD gennum, int level, struct PARSE_CONTEXT* pContext = NULL); + CPDF_Object* GetObjectByStrict(CPDF_IndirectObjects* pObjList, FX_DWORD objnum, FX_DWORD gennum, struct PARSE_CONTEXT* pContext = NULL); int GetDirectNum(); @@ -302,6 +302,8 @@ public: CFX_ByteString GetNextWord(FX_BOOL& bIsNumber); protected: + static const int kParserMaxRecursionDepth = 64; + static int s_CurrentRecursionDepth; virtual FX_BOOL GetNextChar(FX_BYTE& ch); @@ -520,7 +522,6 @@ public: return m_dwFirstPageNo; } protected: - CPDF_Document* m_pDocument; CPDF_SyntaxParser m_Syntax; diff --git a/core/include/fxcrt/fx_basic.h b/core/include/fxcrt/fx_basic.h index 1c828cea48..b63327c9c2 100644 --- a/core/include/fxcrt/fx_basic.h +++ b/core/include/fxcrt/fx_basic.h @@ -1414,6 +1414,21 @@ protected: CFX_DataFilter* m_pDestFilter; }; + +template +class CFX_AutoRestorer { +public: + explicit CFX_AutoRestorer(T* location) { + m_Location = location; + m_OldValue = *location; + } + ~CFX_AutoRestorer() { *m_Location = m_OldValue; } + +private: + T* m_Location; + T m_OldValue; +}; + template class CFX_SmartPointer { diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp index b759cc8a70..f50d5eeb13 100644 --- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp +++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp @@ -12,7 +12,6 @@ #include #include -#define _PARSER_OBJECT_LEVLE_ 64 extern const FX_LPCSTR _PDF_CharType; FX_BOOL IsSignatureDict(const CPDF_Dictionary* pDict) { @@ -43,6 +42,7 @@ static int _CompareFileSize(const void* p1, const void* p2) } return 0; } + CPDF_Parser::CPDF_Parser() { m_pDocument = NULL; @@ -1221,7 +1221,7 @@ CPDF_Object* CPDF_Parser::ParseIndirectObject(CPDF_IndirectObjects* pObjList, FX FX_DWORD thisoff = syntax.GetDirectNum(); if (thisnum == objnum) { syntax.RestorePos(offset + thisoff); - pRet = syntax.GetObject(pObjList, 0, 0, 0, pContext); + pRet = syntax.GetObject(pObjList, 0, 0, pContext); break; } n --; @@ -1400,7 +1400,7 @@ CPDF_Object* CPDF_Parser::ParseIndirectObjectAt(CPDF_IndirectObjects* pObjList, m_Syntax.RestorePos(SavedPos); return NULL; } - CPDF_Object* pObj = m_Syntax.GetObject(pObjList, objnum, parser_gennum, 0, pContext); + CPDF_Object* pObj = m_Syntax.GetObject(pObjList, objnum, parser_gennum, pContext); FX_FILESIZE endOffset = m_Syntax.SavePos(); CFX_ByteString bsWord = m_Syntax.GetKeyword(); if (bsWord == FX_BSTRC("endobj")) { @@ -1441,7 +1441,7 @@ CPDF_Object* CPDF_Parser::ParseIndirectObjectAtByStrict(CPDF_IndirectObjects* pO m_Syntax.RestorePos(SavedPos); return NULL; } - CPDF_Object* pObj = m_Syntax.GetObjectByStrict(pObjList, objnum, gennum, 0, pContext); + CPDF_Object* pObj = m_Syntax.GetObjectByStrict(pObjList, objnum, gennum, pContext); if (pResultPos) { *pResultPos = m_Syntax.m_Pos; } @@ -1680,6 +1680,10 @@ FX_DWORD CPDF_Parser::LoadLinearizedMainXRefTable() m_Syntax.m_MetadataObjnum = dwSaveMetadataObjnum; return PDFPARSE_ERROR_SUCCESS; } + +// static +int CPDF_SyntaxParser::s_CurrentRecursionDepth = 0; + CPDF_SyntaxParser::CPDF_SyntaxParser() { m_pFileAccess = NULL; @@ -2053,9 +2057,10 @@ CFX_ByteString CPDF_SyntaxParser::GetKeyword() GetNextWord(); return CFX_ByteString((FX_LPCSTR)m_WordBuffer, m_WordSize); } -CPDF_Object* CPDF_SyntaxParser::GetObject(CPDF_IndirectObjects* pObjList, FX_DWORD objnum, FX_DWORD gennum, FX_INT32 level, PARSE_CONTEXT* pContext, FX_BOOL bDecrypt) +CPDF_Object* CPDF_SyntaxParser::GetObject(CPDF_IndirectObjects* pObjList, FX_DWORD objnum, FX_DWORD gennum, PARSE_CONTEXT* pContext, FX_BOOL bDecrypt) { - if (level > _PARSER_OBJECT_LEVLE_) { + CFX_AutoRestorer restorer(&s_CurrentRecursionDepth); + if (++s_CurrentRecursionDepth > kParserMaxRecursionDepth) { return NULL; } FX_FILESIZE SavedPos = m_Pos; @@ -2140,7 +2145,7 @@ CPDF_Object* CPDF_SyntaxParser::GetObject(CPDF_IndirectObjects* pObjList, FX_DWO } CPDF_Array* pArray = CPDF_Array::Create(); while (1) { - CPDF_Object* pObj = GetObject(pObjList, objnum, gennum, level + 1); + CPDF_Object* pObj = GetObject(pObjList, objnum, gennum); if (pObj == NULL) { return pArray; } @@ -2192,7 +2197,7 @@ CPDF_Object* CPDF_SyntaxParser::GetObject(CPDF_IndirectObjects* pObjList, FX_DWO if (key == FX_BSTRC("/Contents")) { dwSignValuePos = m_Pos; } - CPDF_Object* pObj = GetObject(pObjList, objnum, gennum, level + 1); + CPDF_Object* pObj = GetObject(pObjList, objnum, gennum); if (pObj == NULL) { continue; } @@ -2207,7 +2212,7 @@ CPDF_Object* CPDF_SyntaxParser::GetObject(CPDF_IndirectObjects* pObjList, FX_DWO if (IsSignatureDict(pDict)) { FX_FILESIZE dwSavePos = m_Pos; m_Pos = dwSignValuePos; - CPDF_Object* pObj = GetObject(pObjList, objnum, gennum, level + 1, NULL, FALSE); + CPDF_Object* pObj = GetObject(pObjList, objnum, gennum, NULL, FALSE); pDict->SetAt(FX_BSTRC("Contents"), pObj); m_Pos = dwSavePos; } @@ -2242,10 +2247,10 @@ CPDF_Object* CPDF_SyntaxParser::GetObject(CPDF_IndirectObjects* pObjList, FX_DWO } return NULL; } -CPDF_Object* CPDF_SyntaxParser::GetObjectByStrict(CPDF_IndirectObjects* pObjList, FX_DWORD objnum, FX_DWORD gennum, - FX_INT32 level, struct PARSE_CONTEXT* pContext) +CPDF_Object* CPDF_SyntaxParser::GetObjectByStrict(CPDF_IndirectObjects* pObjList, FX_DWORD objnum, FX_DWORD gennum, struct PARSE_CONTEXT* pContext) { - if (level > _PARSER_OBJECT_LEVLE_) { + CFX_AutoRestorer restorer(&s_CurrentRecursionDepth); + if (++s_CurrentRecursionDepth > kParserMaxRecursionDepth) { return NULL; } FX_FILESIZE SavedPos = m_Pos; @@ -2322,7 +2327,7 @@ CPDF_Object* CPDF_SyntaxParser::GetObjectByStrict(CPDF_IndirectObjects* pObjList } CPDF_Array* pArray = CPDF_Array::Create(); while (1) { - CPDF_Object* pObj = GetObject(pObjList, objnum, gennum, level + 1); + CPDF_Object* pObj = GetObject(pObjList, objnum, gennum); if (pObj == NULL) { if (m_WordBuffer[0] == ']') { return pArray; @@ -2368,7 +2373,7 @@ CPDF_Object* CPDF_SyntaxParser::GetObjectByStrict(CPDF_IndirectObjects* pObjList continue; } key = PDF_NameDecode(key); - CPDF_Object* pObj = GetObject(pObjList, objnum, gennum, level + 1); + CPDF_Object* pObj = GetObject(pObjList, objnum, gennum); if (pObj == NULL) { if (pDict) pDict->Release(); diff --git a/fpdfsdk/src/fpdfview_embeddertest.cpp b/fpdfsdk/src/fpdfview_embeddertest.cpp index ee32727ace..14a6532a58 100644 --- a/fpdfsdk/src/fpdfview_embeddertest.cpp +++ b/fpdfsdk/src/fpdfview_embeddertest.cpp @@ -177,3 +177,8 @@ TEST_F(FPDFViewEmbeddertest, NamedDestsByName) { dest = FPDF_GetNamedDestByName(document(), "Bogus"); EXPECT_EQ(nullptr, dest); } + +// The following tests pass if the document opens without crashing. +TEST_F(FPDFViewEmbeddertest, Crashers) { + EXPECT_TRUE(OpenDocument("testing/resources/bug_451830.pdf")); +} diff --git a/testing/resources/bug_451830.pdf b/testing/resources/bug_451830.pdf new file mode 100644 index 0000000000..f209bb3ed1 --- /dev/null +++ b/testing/resources/bug_451830.pdf @@ -0,0 +1,14 @@ +%PDF-1.2 +%âãÏÓ +7 0 obj << + /Type /Font +trailer +<> +endstream +endobj +%%EOF -- cgit v1.2.3