summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNicolas Pena <npm@chromium.org>2017-02-10 15:47:50 -0500
committerNicolas Pena <npm@chromium.org>2017-02-10 15:47:50 -0500
commit6058ea2afb83b07834bd0fcb275c2934a60ffcaa (patch)
tree7beedef4057ae0549158c5bf9462a33b12d711a0
parentbc8dcc3ede286fbcaac3f741c379297cffff0eea (diff)
downloadpdfium-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.cpp2
-rw-r--r--core/fpdfapi/page/cpdf_streamparser.cpp19
-rw-r--r--core/fpdfapi/page/cpdf_streamparser.h3
-rw-r--r--testing/libfuzzer/pdf_streamparser_fuzzer.cc3
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;