Fix more integer overflows inside ReadPageHintTable().

BUG=637119

Review-Url: https://codereview.chromium.org/2268693003

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<uint32_t>(nStreamOffset)) {
     FX_SAFE_UINT32 safeLoc = pdfium::base::checked_cast<uint32_t>(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<const uint8_t*>(pBuf);
   size_t temp_size = size;
   while (temp_size) {