From 7da24e66c6e78a7675697ecec641e3802ff722ca Mon Sep 17 00:00:00 2001 From: thestig Date: Mon, 22 Aug 2016 17:47:08 -0700 Subject: Fix more integer overflows inside ReadPageHintTable(). BUG=637119 Review-Url: https://codereview.chromium.org/2268693003 --- core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp | 26 ++++++++++++++------------ core/fxcrt/fx_basic_buffer.cpp | 27 +++++++++++++-------------- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp b/core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp index f1e74dc6bc..18a7b8f079 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp @@ -59,12 +59,12 @@ bool CPDF_HintTables::ReadPageHintTable(CFX_BitStream* hStream) { return false; // Item 1: The least number of objects in a page. - uint32_t dwObjLeastNum = hStream->GetBits(32); + const uint32_t dwObjLeastNum = hStream->GetBits(32); if (!dwObjLeastNum) return FALSE; // Item 2: The location of the first page's page object. - uint32_t dwFirstObjLoc = hStream->GetBits(32); + const uint32_t dwFirstObjLoc = hStream->GetBits(32); if (dwFirstObjLoc > static_cast(nStreamOffset)) { FX_SAFE_UINT32 safeLoc = pdfium::base::checked_cast(nStreamLen); safeLoc += dwFirstObjLoc; @@ -79,18 +79,18 @@ bool CPDF_HintTables::ReadPageHintTable(CFX_BitStream* hStream) { // Item 3: The number of bits needed to represent the difference // between the greatest and least number of objects in a page. - uint32_t dwDeltaObjectsBits = hStream->GetBits(16); + const uint32_t dwDeltaObjectsBits = hStream->GetBits(16); if (!dwDeltaObjectsBits) return FALSE; // Item 4: The least length of a page in bytes. - uint32_t dwPageLeastLen = hStream->GetBits(32); + const uint32_t dwPageLeastLen = hStream->GetBits(32); if (!dwPageLeastLen) return FALSE; // Item 5: The number of bits needed to represent the difference // between the greatest and least length of a page, in bytes. - uint32_t dwDeltaPageLenBits = hStream->GetBits(16); + const uint32_t dwDeltaPageLenBits = hStream->GetBits(16); if (!dwDeltaPageLenBits) return FALSE; @@ -99,11 +99,11 @@ bool CPDF_HintTables::ReadPageHintTable(CFX_BitStream* hStream) { // Item 10: The number of bits needed to represent the greatest // number of shared object references. - uint32_t dwSharedObjBits = hStream->GetBits(16); + const uint32_t dwSharedObjBits = hStream->GetBits(16); // Item 11: The number of bits needed to represent the numerically // greatest shared object identifier used by the pages. - uint32_t dwSharedIdBits = hStream->GetBits(16); + const uint32_t dwSharedIdBits = hStream->GetBits(16); if (!dwSharedObjBits) return FALSE; @@ -111,18 +111,20 @@ bool CPDF_HintTables::ReadPageHintTable(CFX_BitStream* hStream) { // the fractional position for each shared object reference. For each // shared object referenced from a page, there is an indication of // where in the page's content stream the object is first referenced. - uint32_t dwSharedNumeratorBits = hStream->GetBits(16); + const uint32_t dwSharedNumeratorBits = hStream->GetBits(16); if (!dwSharedIdBits) return FALSE; // Item 13: Skip Item 13 which has 16 bits. hStream->SkipBits(16); - // The maximum number of bits allowed to represent the greatest number of - // shared object references. 2^39 should be more than enough. - constexpr uint32_t kMaxSharedObjBits = 39; - if (dwSharedObjBits > kMaxSharedObjBits) + // Sanity check values from the page table header. 2^|kMaxBits| should be more + // than enough to represent most of the values here. + constexpr uint32_t kMaxBits = 34; + if (dwSharedObjBits > kMaxBits || dwDeltaObjectsBits > kMaxBits || + dwSharedIdBits > kMaxBits) { return false; + } const int nPages = GetNumberOfPages(); if (nPages < 1 || nPages >= FPDF_PAGE_MAX_NUM) diff --git a/core/fxcrt/fx_basic_buffer.cpp b/core/fxcrt/fx_basic_buffer.cpp index 91ca6dad7c..5ee1a659e5 100644 --- a/core/fxcrt/fx_basic_buffer.cpp +++ b/core/fxcrt/fx_basic_buffer.cpp @@ -187,24 +187,24 @@ void CFX_BitStream::Init(const uint8_t* pData, uint32_t dwSize) { m_BitSize = dwSize * 8; m_BitPos = 0; } + void CFX_BitStream::ByteAlign() { - int mod = m_BitPos % 8; - if (mod == 0) { - return; - } - m_BitPos += 8 - mod; + m_BitPos = (m_BitPos + 7) & ~7; } + uint32_t CFX_BitStream::GetBits(uint32_t nBits) { - if (nBits > m_BitSize || m_BitPos + nBits > m_BitSize) { + if (nBits > m_BitSize || m_BitPos + nBits > m_BitSize) return 0; - } + if (nBits == 1) { int bit = (m_pData[m_BitPos / 8] & (1 << (7 - m_BitPos % 8))) ? 1 : 0; m_BitPos++; return bit; } + uint32_t byte_pos = m_BitPos / 8; - uint32_t bit_pos = m_BitPos % 8, bit_left = nBits; + uint32_t bit_pos = m_BitPos % 8; + uint32_t bit_left = nBits; uint32_t result = 0; if (bit_pos) { if (8 - bit_pos >= bit_left) { @@ -220,9 +220,8 @@ uint32_t CFX_BitStream::GetBits(uint32_t nBits) { bit_left -= 8; result |= m_pData[byte_pos++] << bit_left; } - if (bit_left) { + if (bit_left) result |= m_pData[byte_pos] >> (8 - bit_left); - } m_BitPos += nBits; return result; } @@ -249,12 +248,12 @@ bool CFX_FileBufferArchive::Flush() { } int32_t CFX_FileBufferArchive::AppendBlock(const void* pBuf, size_t size) { - if (!pBuf || size < 1) { + if (!pBuf || size < 1) return 0; - } - if (!m_pBuffer) { + + if (!m_pBuffer) m_pBuffer.reset(FX_Alloc(uint8_t, kBufSize)); - } + const uint8_t* buffer = reinterpret_cast(pBuf); size_t temp_size = size; while (temp_size) { -- cgit v1.2.3