From bc0ca1ec9b157ab8773c9043725c7422f7c1a57c Mon Sep 17 00:00:00 2001 From: Ryan Harrison Date: Thu, 31 Aug 2017 11:57:14 -0400 Subject: Prevent duplicate parses of same data, in the same recursive descent When parsing if there is a loop in the data being parsed, the recursions will just keep cycling until it exhausts memory and crashes. This CL introduces a parsed set, which a reference to is passed down the descent. If the data being parsed at a specific stage of the descent is already in the parsed set, then the parse returns at that point. BUG=chromium:759224 Change-Id: I1dca73d81020099dec03fd49aaa44cdcdf38e17e Reviewed-on: https://pdfium-review.googlesource.com/12470 Reviewed-by: Tom Sepez Commit-Queue: Ryan Harrison --- core/fpdfapi/page/cpdf_contentparser.cpp | 7 ++++--- core/fpdfapi/page/cpdf_contentparser.h | 6 +++++- core/fpdfapi/page/cpdf_form.cpp | 16 +++++++++++----- core/fpdfapi/page/cpdf_form.h | 9 +++++++-- core/fpdfapi/page/cpdf_streamcontentparser.cpp | 13 +++++++++---- core/fpdfapi/page/cpdf_streamcontentparser.h | 5 +++-- 6 files changed, 39 insertions(+), 17 deletions(-) diff --git a/core/fpdfapi/page/cpdf_contentparser.cpp b/core/fpdfapi/page/cpdf_contentparser.cpp index 061ec74de8..3032f2cd01 100644 --- a/core/fpdfapi/page/cpdf_contentparser.cpp +++ b/core/fpdfapi/page/cpdf_contentparser.cpp @@ -73,7 +73,7 @@ void CPDF_ContentParser::Start(CPDF_Form* pForm, CPDF_AllStates* pGraphicStates, const CFX_Matrix* pParentMatrix, CPDF_Type3Char* pType3Char, - int level) { + std::set* parsedSet) { m_pType3Char = pType3Char; m_pObjectHolder = pForm; m_bForm = true; @@ -101,7 +101,7 @@ void CPDF_ContentParser::Start(CPDF_Form* pForm, m_pParser = pdfium::MakeUnique( pForm->m_pDocument.Get(), pForm->m_pPageResources.Get(), pForm->m_pResources.Get(), pParentMatrix, pForm, pResources, form_bbox, - pGraphicStates, level); + pGraphicStates, parsedSet); m_pParser->GetCurStates()->m_CTM = form_matrix; m_pParser->GetCurStates()->m_ParentMatrix = form_matrix; if (ClipPath.HasRef()) { @@ -169,11 +169,12 @@ void CPDF_ContentParser::Continue(IFX_PauseIndicator* pPause) { } if (m_InternalStage == STAGE_PARSE) { if (!m_pParser) { + m_parsedSet = pdfium::MakeUnique>(); m_pParser = pdfium::MakeUnique( m_pObjectHolder->m_pDocument.Get(), m_pObjectHolder->m_pPageResources.Get(), nullptr, nullptr, m_pObjectHolder.Get(), m_pObjectHolder->m_pResources.Get(), - m_pObjectHolder->m_BBox, nullptr, 0); + m_pObjectHolder->m_BBox, nullptr, m_parsedSet.get()); m_pParser->GetCurStates()->m_ColorState.SetDefault(); } if (m_CurrentOffset >= m_Size) { diff --git a/core/fpdfapi/page/cpdf_contentparser.h b/core/fpdfapi/page/cpdf_contentparser.h index 1ae6efe2d7..b201f95df8 100644 --- a/core/fpdfapi/page/cpdf_contentparser.h +++ b/core/fpdfapi/page/cpdf_contentparser.h @@ -8,6 +8,7 @@ #define CORE_FPDFAPI_PAGE_CPDF_CONTENTPARSER_H_ #include +#include #include #include "core/fpdfapi/page/cpdf_pageobjectholder.h" @@ -37,7 +38,7 @@ class CPDF_ContentParser { CPDF_AllStates* pGraphicStates, const CFX_Matrix* pParentMatrix, CPDF_Type3Char* pType3Char, - int level); + std::set* parsedSet); void Continue(IFX_PauseIndicator* pPause); private: @@ -58,6 +59,9 @@ class CPDF_ContentParser { uint8_t* m_pData; uint32_t m_Size; uint32_t m_CurrentOffset; + std::unique_ptr> m_parsedSet; + // m_pParser has a reference to m_parsedSet, so must be below and thus + // destroyed first. std::unique_ptr m_pParser; }; diff --git a/core/fpdfapi/page/cpdf_form.cpp b/core/fpdfapi/page/cpdf_form.cpp index c5d9cbffcf..12403a4cc9 100644 --- a/core/fpdfapi/page/cpdf_form.cpp +++ b/core/fpdfapi/page/cpdf_form.cpp @@ -35,23 +35,29 @@ CPDF_Form::~CPDF_Form() {} void CPDF_Form::StartParse(CPDF_AllStates* pGraphicStates, const CFX_Matrix* pParentMatrix, CPDF_Type3Char* pType3Char, - int level) { + std::set* parsedSet) { if (m_ParseState == CONTENT_PARSED || m_ParseState == CONTENT_PARSING) return; + if (!parsedSet) { + if (!m_ParsedSet) + m_ParsedSet = pdfium::MakeUnique>(); + parsedSet = m_ParsedSet.get(); + } + m_pParser = pdfium::MakeUnique(); - m_pParser->Start(this, pGraphicStates, pParentMatrix, pType3Char, level); + m_pParser->Start(this, pGraphicStates, pParentMatrix, pType3Char, parsedSet); m_ParseState = CONTENT_PARSING; } void CPDF_Form::ParseContent() { - ParseContentWithParams(nullptr, nullptr, nullptr, 0); + ParseContentWithParams(nullptr, nullptr, nullptr, nullptr); } void CPDF_Form::ParseContentWithParams(CPDF_AllStates* pGraphicStates, const CFX_Matrix* pParentMatrix, CPDF_Type3Char* pType3Char, - int level) { - StartParse(pGraphicStates, pParentMatrix, pType3Char, level); + std::set* parsedSet) { + StartParse(pGraphicStates, pParentMatrix, pType3Char, parsedSet); ContinueParse(nullptr); } diff --git a/core/fpdfapi/page/cpdf_form.h b/core/fpdfapi/page/cpdf_form.h index 2cce800f23..c5285a1d01 100644 --- a/core/fpdfapi/page/cpdf_form.h +++ b/core/fpdfapi/page/cpdf_form.h @@ -7,6 +7,9 @@ #ifndef CORE_FPDFAPI_PAGE_CPDF_FORM_H_ #define CORE_FPDFAPI_PAGE_CPDF_FORM_H_ +#include +#include + #include "core/fpdfapi/page/cpdf_pageobjectholder.h" class CPDF_Document; @@ -28,13 +31,15 @@ class CPDF_Form : public CPDF_PageObjectHolder { void ParseContentWithParams(CPDF_AllStates* pGraphicStates, const CFX_Matrix* pParentMatrix, CPDF_Type3Char* pType3Char, - int level); + std::set* parsedSet); private: void StartParse(CPDF_AllStates* pGraphicStates, const CFX_Matrix* pParentMatrix, CPDF_Type3Char* pType3Char, - int level); + std::set* parsedSet); + + std::unique_ptr> m_ParsedSet; }; #endif // CORE_FPDFAPI_PAGE_CPDF_FORM_H_ diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp index 7bd6b50123..3755b2985a 100644 --- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp +++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp @@ -37,6 +37,7 @@ #include "core/fxge/cfx_graphstatedata.h" #include "third_party/base/logging.h" #include "third_party/base/ptr_util.h" +#include "third_party/base/stl_util.h" namespace { @@ -243,13 +244,13 @@ CPDF_StreamContentParser::CPDF_StreamContentParser( CPDF_Dictionary* pResources, const CFX_FloatRect& rcBBox, CPDF_AllStates* pStates, - int level) + std::set* parsedSet) : m_pDocument(pDocument), m_pPageResources(pPageResources), m_pParentResources(pParentResources), m_pResources(pResources), m_pObjectHolder(pObjHolder), - m_Level(level), + m_ParsedSet(parsedSet), m_BBox(rcBBox), m_ParamStartPos(0), m_ParamCount(0), @@ -776,7 +777,7 @@ void CPDF_StreamContentParser::AddForm(CPDF_Stream* pStream) { status.m_ColorState = m_pCurStates->m_ColorState; status.m_TextState = m_pCurStates->m_TextState; pFormObj->m_pForm->ParseContentWithParams(&status, nullptr, nullptr, - m_Level + 1); + m_ParsedSet.Get()); if (!m_pObjectHolder->BackgroundAlphaNeeded() && pFormObj->m_pForm->BackgroundAlphaNeeded()) { m_pObjectHolder->SetBackgroundAlphaNeeded(true); @@ -1507,9 +1508,13 @@ void CPDF_StreamContentParser::AddPathObject(int FillType, bool bStroke) { uint32_t CPDF_StreamContentParser::Parse(const uint8_t* pData, uint32_t dwSize, uint32_t max_cost) { - if (m_Level > kMaxFormLevel) + if (m_ParsedSet->size() > kMaxFormLevel || + pdfium::ContainsKey(*m_ParsedSet, pData)) return dwSize; + pdfium::ScopedSetInsertion scopedInsert(m_ParsedSet.Get(), + pData); + uint32_t InitObjCount = m_pObjectHolder->GetPageObjectList()->size(); CPDF_StreamParser syntax(pData, dwSize, m_pDocument->GetByteStringPool()); CPDF_StreamParserAutoClearer auto_clearer(&m_pSyntax, &syntax); diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.h b/core/fpdfapi/page/cpdf_streamcontentparser.h index 5cbe0ce734..a027129c31 100644 --- a/core/fpdfapi/page/cpdf_streamcontentparser.h +++ b/core/fpdfapi/page/cpdf_streamcontentparser.h @@ -9,6 +9,7 @@ #include #include +#include #include #include "core/fpdfapi/page/cpdf_contentmark.h" @@ -39,7 +40,7 @@ class CPDF_StreamContentParser { CPDF_Dictionary* pResources, const CFX_FloatRect& rcBBox, CPDF_AllStates* pAllStates, - int level); + std::set* parsedSet); ~CPDF_StreamContentParser(); uint32_t Parse(const uint8_t* pData, uint32_t dwSize, uint32_t max_cost); @@ -198,7 +199,7 @@ class CPDF_StreamContentParser { CFX_UnownedPtr m_pParentResources; CFX_UnownedPtr m_pResources; CFX_UnownedPtr m_pObjectHolder; - const int m_Level; + CFX_UnownedPtr> m_ParsedSet; CFX_Matrix m_mtContentToUser; const CFX_FloatRect m_BBox; ContentParam m_ParamBuf[kParamBufSize]; -- cgit v1.2.3