Handle another integer overflow in ReadPageHintTable().

core/fpdfapi/fpdf_parser/cpdf_hint_tables.cpp

 // Copyright 2016 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "core/fpdfapi/fpdf_parser/cpdf_hint_tables.h"
 
+#include <limits>
+
 #include "core/fpdfapi/fpdf_parser/include/cpdf_array.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_data_avail.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_document.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_stream.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_stream_acc.h"
 #include "core/fxcrt/include/fx_safe_types.h"
 
 namespace {
 
@@ skipping 23 matching lines @@
       szArray[index] > szArray[index + 1]) {
     return 0;
   }
   return szArray[index + 1] - szArray[index];
 }
 
 bool CPDF_HintTables::ReadPageHintTable(CFX_BitStream* hStream) {
   if (!hStream || hStream->IsEOF())
     return false;
 
   int nStreamOffset = ReadPrimaryHintStreamOffset();

[Tom Sepez, 2016/09/01 16:08:04]

nit: can these two be int32_t's since presumably the data is coming from 4 bytes
in a stream somewhere?

[Lei Zhang, 2016/09/01 17:49:19]

No, they are from CPDF_Number::GetInteger() and not 4 byte entry in the header.

+  if (nStreamOffset < 0)
+    return false;
+
   int nStreamLen = ReadPrimaryHintStreamLength();
-  if (nStreamOffset < 0 || nStreamLen < 1)
+  if (nStreamLen < 1 ||
+      !pdfium::base::IsValueInRangeForNumericType<FX_FILESIZE>(nStreamLen)) {

[Tom Sepez, 2016/09/01 16:08:04]

Ok, but I thought FX_FILESIZE was signed so we could return -1 as an error code
... did you have a situation where this could trip?

[Lei Zhang, 2016/09/01 17:49:18]

It may not be obvious if an int fits inside a FX_FILESIZE.

As is FX_FILESIZE can either be int32_t or off_t, so this probably won't trip.
If it does ever trip, I'd like it to trip here early and return false, rather
than trip with a checked_cast on line 76.

     return false;
+  }
 
   const uint32_t kHeaderSize = 288;
   if (hStream->BitsRemaining() < kHeaderSize)
     return false;
 
   // Item 1: The least number of objects in a page.
   const uint32_t dwObjLeastNum = hStream->GetBits(32);
   if (!dwObjLeastNum)
-    return FALSE;
+    return false;
 
   // Item 2: The location of the first page's page object.
   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);
+    FX_SAFE_FILESIZE safeLoc = nStreamLen;
     safeLoc += dwFirstObjLoc;
     if (!safeLoc.IsValid())
       return false;
-    m_szFirstPageObjOffset =
-        pdfium::base::checked_cast<FX_FILESIZE>(safeLoc.ValueOrDie());
+    m_szFirstPageObjOffset = safeLoc.ValueOrDie();
   } else {
-    m_szFirstPageObjOffset =
-        pdfium::base::checked_cast<FX_FILESIZE>(dwFirstObjLoc);
+    if (!pdfium::base::IsValueInRangeForNumericType<FX_FILESIZE>(dwFirstObjLoc))
+      return false;
+    m_szFirstPageObjOffset = dwFirstObjLoc;
   }
 
   // Item 3: The number of bits needed to represent the difference
   // between the greatest and least number of objects in a page.
   const uint32_t dwDeltaObjectsBits = hStream->GetBits(16);
   if (!dwDeltaObjectsBits)
-    return FALSE;
+    return false;
 
   // Item 4: The least length of a page in bytes.
   const uint32_t dwPageLeastLen = hStream->GetBits(32);
   if (!dwPageLeastLen)
-    return FALSE;
+    return false;
 
   // Item 5: The number of bits needed to represent the difference
   // between the greatest and least length of a page, in bytes.
   const uint32_t dwDeltaPageLenBits = hStream->GetBits(16);
   if (!dwDeltaPageLenBits)
-    return FALSE;
+    return false;
 
   // Skip Item 6, 7, 8, 9 total 96 bits.
   hStream->SkipBits(96);
 
   // Item 10: The number of bits needed to represent the greatest
   // number of shared object references.
   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.
   const uint32_t dwSharedIdBits = hStream->GetBits(16);
   if (!dwSharedObjBits)
-    return FALSE;
+    return false;
 
   // Item 12: The number of bits needed to represent the numerator of
   // 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.
   const uint32_t dwSharedNumeratorBits = hStream->GetBits(16);
   if (!dwSharedIdBits)
-    return FALSE;
+    return false;
 
   // Item 13: Skip Item 13 which has 16 bits.
   hStream->SkipBits(16);
 
   // 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 ||
       dwDeltaPageLenBits > kMaxBits || dwSharedIdBits > kMaxBits) {
     return false;
@@ skipping 378 matching lines @@
 }
 
 int CPDF_HintTables::ReadPrimaryHintStream(int index) const {
   CPDF_Array* pRange = m_pLinearizedDict->GetArrayBy("H");
   if (!pRange)
     return -1;
 
   CPDF_Object* pStreamLen = pRange->GetDirectObjectAt(index);
   return pStreamLen ? pStreamLen->GetInteger() : -1;
 }