diff options
author | Nicolas Pena <npm@chromium.org> | 2017-02-10 15:47:50 -0500 |
---|---|---|
committer | Nicolas Pena <npm@chromium.org> | 2017-02-10 15:47:50 -0500 |
commit | 6058ea2afb83b07834bd0fcb275c2934a60ffcaa (patch) | |
tree | 7beedef4057ae0549158c5bf9462a33b12d711a0 | |
parent | bc8dcc3ede286fbcaac3f741c379297cffff0eea (diff) | |
download | pdfium-6058ea2afb83b07834bd0fcb275c2934a60ffcaa.tar.xz |
[M57] Limit parsing recursion levels in CPDF_StreamParser
We currently only limit the array recursion levels. This recursion
level may also be reset when parsing. This is insufficient to protect
against stack overflows.
BUG=681920
TBR=dsinclair@chromium.org, tsepez@chromium.org
Change-Id: I69bd0c912fb45c0e68b9b9fa961d43f0adc9bdd3
Reviewed-on: https://pdfium-review.googlesource.com/2434
Commit-Queue: Nicolás Peña <npm@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
(cherry picked from commit 6438c4f36da162f72e0d53e8fff45cd6687b7f5c)
Review-Url: https://codereview.chromium.org/2686193003 .
-rw-r--r-- | core/fpdfapi/page/cpdf_streamcontentparser.cpp | 2 | ||||
-rw-r--r-- | core/fpdfapi/page/cpdf_streamparser.cpp | 19 | ||||
-rw-r--r-- | core/fpdfapi/page/cpdf_streamparser.h | 3 | ||||
-rw-r--r-- | testing/libfuzzer/pdf_streamparser_fuzzer.cc | 3 |
4 files changed, 15 insertions, 12 deletions
diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp index 141442bb28..0b41979f4a 100644 --- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp +++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp @@ -644,7 +644,7 @@ void CPDF_StreamContentParser::Handle_BeginImage() { } CFX_ByteString key((const FX_CHAR*)m_pSyntax->GetWordBuf() + 1, m_pSyntax->GetWordSize() - 1); - auto pObj = m_pSyntax->ReadNextObject(false, 0); + auto pObj = m_pSyntax->ReadNextObject(false, false, 0); if (!key.IsEmpty()) { uint32_t dwObjNum = pObj ? pObj->GetObjNum() : 0; if (dwObjNum) diff --git a/core/fpdfapi/page/cpdf_streamparser.cpp b/core/fpdfapi/page/cpdf_streamparser.cpp index e26de605b0..f8f92e3391 100644 --- a/core/fpdfapi/page/cpdf_streamparser.cpp +++ b/core/fpdfapi/page/cpdf_streamparser.cpp @@ -29,7 +29,7 @@ namespace { -const uint32_t kMaxNestedArrayLevel = 512; +const uint32_t kMaxNestedParsingLevel = 512; const uint32_t kMaxWordBuffer = 256; const FX_STRSIZE kMaxStringLength = 32767; @@ -256,7 +256,7 @@ CPDF_StreamParser::SyntaxType CPDF_StreamParser::ParseNextElement() { if (PDFCharIsDelimiter(ch) && ch != '/') { m_Pos--; - m_pLastObj = ReadNextObject(false, 0); + m_pLastObj = ReadNextObject(false, false, 0); return Others; } @@ -306,10 +306,12 @@ CPDF_StreamParser::SyntaxType CPDF_StreamParser::ParseNextElement() { std::unique_ptr<CPDF_Object> CPDF_StreamParser::ReadNextObject( bool bAllowNestedArray, - uint32_t dwInArrayLevel) { + bool bInArray, + uint32_t dwRecursionLevel) { bool bIsNumber; + // Must get the next word before returning to avoid infinite loops. GetNextWord(bIsNumber); - if (!m_WordSize) + if (!m_WordSize || dwRecursionLevel > kMaxNestedParsingLevel) return nullptr; if (bIsNumber) { @@ -345,7 +347,8 @@ std::unique_ptr<CPDF_Object> CPDF_StreamParser::ReadNextObject( CFX_ByteString key = PDF_NameDecode(CFX_ByteStringC(m_WordBuffer + 1, m_WordSize - 1)); - std::unique_ptr<CPDF_Object> pObj = ReadNextObject(true, 0); + std::unique_ptr<CPDF_Object> pObj = + ReadNextObject(true, bInArray, dwRecursionLevel + 1); if (!pObj) return nullptr; @@ -356,15 +359,13 @@ std::unique_ptr<CPDF_Object> CPDF_StreamParser::ReadNextObject( } if (first_char == '[') { - if ((!bAllowNestedArray && dwInArrayLevel) || - dwInArrayLevel > kMaxNestedArrayLevel) { + if ((!bAllowNestedArray && bInArray)) return nullptr; - } auto pArray = pdfium::MakeUnique<CPDF_Array>(); while (1) { std::unique_ptr<CPDF_Object> pObj = - ReadNextObject(bAllowNestedArray, dwInArrayLevel + 1); + ReadNextObject(bAllowNestedArray, true, dwRecursionLevel + 1); if (pObj) { pArray->Add(std::move(pObj)); continue; diff --git a/core/fpdfapi/page/cpdf_streamparser.h b/core/fpdfapi/page/cpdf_streamparser.h index ce01dd04ee..a4d2798032 100644 --- a/core/fpdfapi/page/cpdf_streamparser.h +++ b/core/fpdfapi/page/cpdf_streamparser.h @@ -34,7 +34,8 @@ class CPDF_StreamParser { void SetPos(uint32_t pos) { m_Pos = pos; } std::unique_ptr<CPDF_Object> GetObject() { return std::move(m_pLastObj); } std::unique_ptr<CPDF_Object> ReadNextObject(bool bAllowNestedArray, - uint32_t dwInArrayLevel); + bool bInArray, + uint32_t dwRecursionLevel); std::unique_ptr<CPDF_Stream> ReadInlineStream( CPDF_Document* pDoc, std::unique_ptr<CPDF_Dictionary> pDict, diff --git a/testing/libfuzzer/pdf_streamparser_fuzzer.cc b/testing/libfuzzer/pdf_streamparser_fuzzer.cc index 5cfa318c60..46113d42c6 100644 --- a/testing/libfuzzer/pdf_streamparser_fuzzer.cc +++ b/testing/libfuzzer/pdf_streamparser_fuzzer.cc @@ -10,7 +10,8 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { CPDF_StreamParser parser(data, size); - while (std::unique_ptr<CPDF_Object> pObj = parser.ReadNextObject(true, 0)) + while (std::unique_ptr<CPDF_Object> pObj = + parser.ReadNextObject(true, false, 0)) continue; return 0; |