From 29d447ae35101675a2a2d8bc1dcfca65de7f3929 Mon Sep 17 00:00:00 2001 From: thestig Date: Thu, 23 Jun 2016 19:57:45 -0700 Subject: Improve hint table validation checks. Check required hint table dictionary entries and make sure they: - Exist. - Are of the right type. Along the way: - Fix FX_atonum() to not have a non-const pass-by-ref param. - Simplify code in CPDF_StreamContentParser. - Make CPDF_Number::IsInteger() a const method. BUG=610555 Review-Url: https://codereview.chromium.org/2095763003 --- core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp | 41 +++++++++++++++++--------- core/fpdfapi/fpdf_parser/cpdf_number.cpp | 13 ++++---- core/fpdfapi/fpdf_parser/include/cpdf_number.h | 4 +-- 3 files changed, 35 insertions(+), 23 deletions(-) (limited to 'core/fpdfapi/fpdf_parser') diff --git a/core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp b/core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp index 5a7a7be544..cc5b529b45 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp @@ -6,6 +6,8 @@ #include "core/fpdfapi/fpdf_parser/include/cpdf_data_avail.h" +#include + #include "core/fpdfapi/fpdf_parser/cpdf_hint_tables.h" #include "core/fpdfapi/fpdf_parser/fpdf_parser_utility.h" #include "core/fpdfapi/fpdf_parser/include/cpdf_array.h" @@ -723,37 +725,48 @@ FX_BOOL CPDF_DataAvail::CheckHintTables(DownloadHints* pHints) { return FALSE; } - if (!pDict->KeyExist("H") || !pDict->KeyExist("O") || !pDict->KeyExist("N")) { + // The actual value is not required here, but validate its existence and type. + CPDF_Number* pFirstPage = ToNumber(pDict->GetDirectObjectBy("O")); + if (!pFirstPage || !pFirstPage->IsInteger()) { + m_docStatus = PDF_DATAAVAIL_ERROR; + return FALSE; + } + + CPDF_Number* pPageCount = ToNumber(pDict->GetDirectObjectBy("N")); + if (!pPageCount || !pPageCount->IsInteger()) { m_docStatus = PDF_DATAAVAIL_ERROR; return FALSE; } - int nPageCount = pDict->GetDirectObjectBy("N")->GetInteger(); + int nPageCount = pPageCount->GetInteger(); if (nPageCount <= 1) { m_docStatus = PDF_DATAAVAIL_DONE; return TRUE; } CPDF_Array* pHintStreamRange = pDict->GetArrayBy("H"); - if (!pHintStreamRange) { + size_t nHintStreamSize = pHintStreamRange ? pHintStreamRange->GetCount() : 0; + if (nHintStreamSize != 2 && nHintStreamSize != 4) { m_docStatus = PDF_DATAAVAIL_ERROR; return FALSE; } - FX_FILESIZE szHSStart = - pHintStreamRange->GetDirectObjectAt(0) - ? pHintStreamRange->GetDirectObjectAt(0)->GetInteger() - : 0; - FX_FILESIZE szHSLength = - pHintStreamRange->GetDirectObjectAt(1) - ? pHintStreamRange->GetDirectObjectAt(1)->GetInteger() - : 0; - if (szHSStart < 0 || szHSLength <= 0) { + for (const CPDF_Object* pArrayObject : *pHintStreamRange) { + const CPDF_Number* pNumber = ToNumber(pArrayObject->GetDirect()); + if (!pNumber || !pNumber->IsInteger()) { + m_docStatus = PDF_DATAAVAIL_ERROR; + return FALSE; + } + } + + FX_FILESIZE szHintStart = pHintStreamRange->GetIntegerAt(0); + FX_FILESIZE szHintLength = pHintStreamRange->GetIntegerAt(1); + if (szHintStart < 0 || szHintLength <= 0) { m_docStatus = PDF_DATAAVAIL_ERROR; return FALSE; } - if (!IsDataAvail(szHSStart, szHSLength, pHints)) + if (!IsDataAvail(szHintStart, szHintLength, pHints)) return FALSE; m_syntaxParser.InitParser(m_pFileRead, m_dwHeaderOffset); @@ -761,7 +774,7 @@ FX_BOOL CPDF_DataAvail::CheckHintTables(DownloadHints* pHints) { std::unique_ptr pHintTables( new CPDF_HintTables(this, pDict)); std::unique_ptr> pHintStream( - ParseIndirectObjectAt(szHSStart, 0)); + ParseIndirectObjectAt(szHintStart, 0)); CPDF_Stream* pStream = ToStream(pHintStream.get()); if (pStream && pHintTables->LoadHintStream(pStream)) m_pHintTables = std::move(pHintTables); diff --git a/core/fpdfapi/fpdf_parser/cpdf_number.cpp b/core/fpdfapi/fpdf_parser/cpdf_number.cpp index a6bace57c9..5012e308ce 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_number.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_number.cpp @@ -6,15 +6,14 @@ #include "core/fpdfapi/fpdf_parser/include/cpdf_number.h" -CPDF_Number::CPDF_Number() : m_bInteger(TRUE), m_Integer(0) {} +CPDF_Number::CPDF_Number() : m_bInteger(true), m_Integer(0) {} -CPDF_Number::CPDF_Number(int value) : m_bInteger(TRUE), m_Integer(value) {} +CPDF_Number::CPDF_Number(int value) : m_bInteger(true), m_Integer(value) {} -CPDF_Number::CPDF_Number(FX_FLOAT value) : m_bInteger(FALSE), m_Float(value) {} +CPDF_Number::CPDF_Number(FX_FLOAT value) : m_bInteger(false), m_Float(value) {} -CPDF_Number::CPDF_Number(const CFX_ByteStringC& str) { - FX_atonum(str, m_bInteger, &m_Integer); -} +CPDF_Number::CPDF_Number(const CFX_ByteStringC& str) + : m_bInteger(FX_atonum(str, &m_Integer)) {} CPDF_Number::~CPDF_Number() {} @@ -47,7 +46,7 @@ const CPDF_Number* CPDF_Number::AsNumber() const { } void CPDF_Number::SetString(const CFX_ByteString& str) { - FX_atonum(str.AsStringC(), m_bInteger, &m_Integer); + m_bInteger = FX_atonum(str.AsStringC(), &m_Integer); } CFX_ByteString CPDF_Number::GetString() const { diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_number.h b/core/fpdfapi/fpdf_parser/include/cpdf_number.h index 7b93a992b1..eaa4009543 100644 --- a/core/fpdfapi/fpdf_parser/include/cpdf_number.h +++ b/core/fpdfapi/fpdf_parser/include/cpdf_number.h @@ -29,12 +29,12 @@ class CPDF_Number : public CPDF_Object { CPDF_Number* AsNumber() override; const CPDF_Number* AsNumber() const override; - FX_BOOL IsInteger() { return m_bInteger; } + bool IsInteger() const { return m_bInteger; } protected: ~CPDF_Number() override; - FX_BOOL m_bInteger; + bool m_bInteger; union { int m_Integer; FX_FLOAT m_Float; -- cgit v1.2.3