From e769ab578af3cf646f6268c992fd9c9dcd494708 Mon Sep 17 00:00:00 2001 From: Ryan Harrison Date: Thu, 7 Jun 2018 17:14:23 +0000 Subject: Revert "Reland "Simplify CPDF_HintsTable."" This reverts commit d89f1bf48f017ab9f56df13299f75a906ed33cd0. Reason for revert: This CL has introduced at least two CF issues chromium:850407,chromium:850440. Additionally there is a number of changes that remove bounds checks, which I think are suspect. BUG=chromium:850407,chromium:850440 Original change's description: > Reland "Simplify CPDF_HintsTable." > > This is a reland of 33591752d2cb14f2e07726ca52afce6efbdc07c9 > > Original change's description: > > Simplify CPDF_HintsTable. > > > > Use CPDF_LinearizedHeader directly. > > > > Change-Id: Id12402ef6e6f92fef68d0932df2e1ccb2dcf06aa > > Reviewed-on: https://pdfium-review.googlesource.com/15770 > > Reviewed-by: Lei Zhang > > Commit-Queue: Lei Zhang > > Change-Id: I2b5425a6533f4ce237f9ae6c483caa517105a5f7 > Reviewed-on: https://pdfium-review.googlesource.com/34130 > Reviewed-by: Lei Zhang > Commit-Queue: Art Snake TBR=thestig@chromium.org,tsepez@chromium.org,art-snake@yandex-team.ru Change-Id: I463b5b1330f809c2cb508cbf46a804b7a11526e4 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://pdfium-review.googlesource.com/34350 Reviewed-by: Ryan Harrison Commit-Queue: Ryan Harrison --- core/fpdfapi/parser/cpdf_hint_tables.cpp | 91 +++++++++++++++++++++++++------- core/fpdfapi/parser/cpdf_hint_tables.h | 8 +++ 2 files changed, 80 insertions(+), 19 deletions(-) diff --git a/core/fpdfapi/parser/cpdf_hint_tables.cpp b/core/fpdfapi/parser/cpdf_hint_tables.cpp index 65beda7e2a..9cd72c7815 100644 --- a/core/fpdfapi/parser/cpdf_hint_tables.cpp +++ b/core/fpdfapi/parser/cpdf_hint_tables.cpp @@ -62,8 +62,15 @@ bool CPDF_HintTables::ReadPageHintTable(CFX_BitStream* hStream) { if (!hStream || hStream->IsEOF()) return false; - const FX_FILESIZE nStreamOffset = m_pLinearized->GetHintStart(); - const uint32_t nStreamLen = m_pLinearized->GetHintLength(); + int nStreamOffset = ReadPrimaryHintStreamOffset(); + if (nStreamOffset < 0) + return false; + + int nStreamLen = ReadPrimaryHintStreamLength(); + if (nStreamLen < 1 || + !pdfium::base::IsValueInRangeForNumericType(nStreamLen)) { + return false; + } const uint32_t kHeaderSize = 288; if (hStream->BitsRemaining() < kHeaderSize) @@ -131,7 +138,7 @@ bool CPDF_HintTables::ReadPageHintTable(CFX_BitStream* hStream) { // Item 13: Skip Item 13 which has 16 bits. hStream->SkipBits(16); - const uint32_t nPages = m_pLinearized->GetPageCount(); + const int nPages = GetNumberOfPages(); if (nPages < 1 || nPages >= FPDF_PAGE_MAX_NUM) return false; @@ -141,7 +148,7 @@ bool CPDF_HintTables::ReadPageHintTable(CFX_BitStream* hStream) { if (!CanReadFromBitStream(hStream, required_bits)) return false; - for (uint32_t i = 0; i < nPages; ++i) { + for (int i = 0; i < nPages; ++i) { FX_SAFE_UINT32 safeDeltaObj = hStream->GetBits(dwDeltaObjectsBits); safeDeltaObj += dwObjLeastNum; if (!safeDeltaObj.IsValid()) @@ -156,7 +163,7 @@ bool CPDF_HintTables::ReadPageHintTable(CFX_BitStream* hStream) { return false; std::vector dwPageLenArray; - for (uint32_t i = 0; i < nPages; ++i) { + for (int i = 0; i < nPages; ++i) { FX_SAFE_UINT32 safePageLen = hStream->GetBits(dwDeltaPageLenBits); safePageLen += dwPageLeastLen; if (!safePageLen.IsValid()) @@ -165,9 +172,15 @@ bool CPDF_HintTables::ReadPageHintTable(CFX_BitStream* hStream) { dwPageLenArray.push_back(safePageLen.ValueOrDie()); } - const FX_FILESIZE nOffsetE = m_pLinearized->GetFirstPageEndOffset(); - const uint32_t nFirstPageNum = m_pLinearized->GetFirstPageNo(); - for (uint32_t i = 0; i < nPages; ++i) { + int nOffsetE = GetEndOfFirstPageOffset(); + if (nOffsetE < 0) + return false; + + int nFirstPageNum = GetFirstPageNumber(); + if (nFirstPageNum < 0 || nFirstPageNum > std::numeric_limits::max() - 1) + return false; + + for (int i = 0; i < nPages; ++i) { if (i == nFirstPageNum) { m_szPageOffsetArray.push_back(m_szFirstPageObjOffset); } else if (i == nFirstPageNum + 1) { @@ -197,12 +210,12 @@ bool CPDF_HintTables::ReadPageHintTable(CFX_BitStream* hStream) { if (!CanReadFromBitStream(hStream, required_bits)) return false; - for (uint32_t i = 0; i < nPages; i++) + for (int i = 0; i < nPages; i++) m_dwNSharedObjsArray.push_back(hStream->GetBits(dwSharedObjBits)); hStream->ByteAlign(); // Array of identifiers, size = nshared_objects. - for (uint32_t i = 0; i < nPages; i++) { + for (int i = 0; i < nPages; i++) { required_bits = dwSharedIdBits; required_bits *= m_dwNSharedObjsArray[i]; if (!CanReadFromBitStream(hStream, required_bits)) @@ -213,7 +226,7 @@ bool CPDF_HintTables::ReadPageHintTable(CFX_BitStream* hStream) { } hStream->ByteAlign(); - for (uint32_t i = 0; i < nPages; i++) { + for (int i = 0; i < nPages; i++) { FX_SAFE_UINT32 safeSize = m_dwNSharedObjsArray[i]; safeSize *= dwSharedNumeratorBits; if (!CanReadFromBitStream(hStream, safeSize)) @@ -238,8 +251,10 @@ bool CPDF_HintTables::ReadSharedObjHintTable(CFX_BitStream* hStream, if (!hStream || hStream->IsEOF()) return false; - const FX_FILESIZE nStreamOffset = m_pLinearized->GetHintStart(); - const uint32_t nStreamLen = m_pLinearized->GetHintLength(); + int nStreamOffset = ReadPrimaryHintStreamOffset(); + int nStreamLen = ReadPrimaryHintStreamLength(); + if (nStreamOffset < 0 || nStreamLen < 1) + return false; FX_SAFE_UINT32 bit_offset = offset; bit_offset *= 8; @@ -289,7 +304,9 @@ bool CPDF_HintTables::ReadSharedObjHintTable(CFX_BitStream* hStream, return false; } - const uint32_t nFirstPageObjNum = m_pLinearized->GetFirstPageObjNum(); + int nFirstPageObjNum = GetFirstPageObjectNumber(); + if (nFirstPageObjNum < 0) + return false; uint32_t dwPrevObjLen = 0; uint32_t dwCurObjLen = 0; @@ -364,9 +381,15 @@ bool CPDF_HintTables::GetPagePos(uint32_t index, *szPageStartPos = m_szPageOffsetArray[index]; *szPageLength = GetItemLength(index, m_szPageOffsetArray); - const uint32_t nFirstPageObjNum = m_pLinearized->GetFirstPageObjNum(); + int nFirstPageObjNum = GetFirstPageObjectNumber(); + if (nFirstPageObjNum < 0) + return false; - const uint32_t dwFirstPageNum = m_pLinearized->GetFirstPageNo(); + int nFirstPageNum = GetFirstPageNumber(); + if (!pdfium::base::IsValueInRangeForNumericType(nFirstPageNum)) + return false; + + uint32_t dwFirstPageNum = static_cast(nFirstPageNum); if (index == dwFirstPageNum) { *dwObjNum = nFirstPageObjNum; return true; @@ -383,7 +406,11 @@ bool CPDF_HintTables::GetPagePos(uint32_t index, } CPDF_DataAvail::DocAvailStatus CPDF_HintTables::CheckPage(uint32_t index) { - if (index == m_pLinearized->GetFirstPageNo()) + int nFirstPageNum = GetFirstPageNumber(); + if (!pdfium::base::IsValueInRangeForNumericType(nFirstPageNum)) + return CPDF_DataAvail::DataError; + + if (index == static_cast(nFirstPageNum)) return CPDF_DataAvail::DataAvailable; uint32_t dwLength = GetItemLength(index, m_szPageOffsetArray); @@ -401,7 +428,9 @@ CPDF_DataAvail::DocAvailStatus CPDF_HintTables::CheckPage(uint32_t index) { for (uint32_t i = 0; i < index; ++i) offset += m_dwNSharedObjsArray[i]; - const uint32_t nFirstPageObjNum = m_pLinearized->GetFirstPageObjNum(); + int nFirstPageObjNum = GetFirstPageObjectNumber(); + if (nFirstPageObjNum < 0) + return CPDF_DataAvail::DataError; uint32_t dwIndex = 0; uint32_t dwObjNum = 0; @@ -431,7 +460,7 @@ CPDF_DataAvail::DocAvailStatus CPDF_HintTables::CheckPage(uint32_t index) { } bool CPDF_HintTables::LoadHintStream(CPDF_Stream* pHintStream) { - if (!pHintStream || !m_pLinearized->HasHintTable()) + if (!pHintStream) return false; CPDF_Dictionary* pDict = pHintStream->GetDict(); @@ -464,3 +493,27 @@ bool CPDF_HintTables::LoadHintStream(CPDF_Stream* pHintStream) { return ReadPageHintTable(&bs) && ReadSharedObjHintTable(&bs, shared_hint_table_offset); } + +int CPDF_HintTables::GetEndOfFirstPageOffset() const { + return static_cast(m_pLinearized->GetFirstPageEndOffset()); +} + +int CPDF_HintTables::GetNumberOfPages() const { + return static_cast(m_pLinearized->GetPageCount()); +} + +int CPDF_HintTables::GetFirstPageObjectNumber() const { + return static_cast(m_pLinearized->GetFirstPageObjNum()); +} + +int CPDF_HintTables::GetFirstPageNumber() const { + return static_cast(m_pLinearized->GetFirstPageNo()); +} + +int CPDF_HintTables::ReadPrimaryHintStreamOffset() const { + return static_cast(m_pLinearized->GetHintStart()); +} + +int CPDF_HintTables::ReadPrimaryHintStreamLength() const { + return static_cast(m_pLinearized->GetHintLength()); +} diff --git a/core/fpdfapi/parser/cpdf_hint_tables.h b/core/fpdfapi/parser/cpdf_hint_tables.h index 3664e1517c..d5feb4638c 100644 --- a/core/fpdfapi/parser/cpdf_hint_tables.h +++ b/core/fpdfapi/parser/cpdf_hint_tables.h @@ -38,6 +38,14 @@ class CPDF_HintTables { bool ReadSharedObjHintTable(CFX_BitStream* hStream, uint32_t offset); private: + // Tests can override. + virtual int GetEndOfFirstPageOffset() const; + virtual int GetNumberOfPages() const; + virtual int GetFirstPageObjectNumber() const; + virtual int GetFirstPageNumber() const; + virtual int ReadPrimaryHintStreamOffset() const; + virtual int ReadPrimaryHintStreamLength() const; + uint32_t GetItemLength(uint32_t index, const std::vector& szArray) const; -- cgit v1.2.3