Merge to M48: 3 commits that fix some hint table loading issues.

TBR=thestig@chromium.org
BUG=566179

Properly land "Fix hint table loading issues."

Original Review URL: https://codereview.chromium.org/1524983002 .

(cherry picked from commit 1eb7477b3e7c5cb7c54ca364810ab9a24acad4f9)

Fix hint table loading issues.

Original Review URL: https://codereview.chromium.org/1523523002 .

(cherry picked from commit be8408f43bcfd69a74007a340a4c034004146c60)

Fix an incorrect check in CPDF_DataAvail::CheckHintTables.

Original Review URL: https://codereview.chromium.org/1504513002 .

(cherry picked from commit 2d16308298f236ae81aa11a53f2d4e25b502dfac)

Review URL: https://codereview.chromium.org/1579643004 .

3 files changed, 73 insertions, 15 deletions

diff --git a/core/include/fxcrt/fx_basic.h b/core/include/fxcrt/fx_basic.h
index c5bb570fca..8a39729fd0 100644
--- a/core/include/fxcrt/fx_basic.h
+++ b/core/include/fxcrt/fx_basic.h
@@ -811,6 +811,12 @@ class CFX_BitStream {
 
   void Rewind() { m_BitPos = 0; }
 
+  FX_DWORD GetPos() const { return m_BitPos; }
+
+  FX_DWORD BitsRemaining() const {
+    return m_BitSize >= m_BitPos ? m_BitSize - m_BitPos : 0;
+  }
+
  protected:
   FX_DWORD m_BitPos;
 
diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
index bf6630bc0d..14e93a0a53 100644
--- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
+++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
@@ -74,6 +74,12 @@ int32_t GetStreamFirst(CPDF_StreamAcc* pObjStream) {
   return pObjStream->GetDict()->GetInteger(FX_BSTRC("First"));
 }
 
+bool CanReadFromBitStream(const CFX_BitStream* hStream,
+                          const FX_SAFE_DWORD& num_bits) {
+  return (num_bits.IsValid() &&
+          hStream->BitsRemaining() >= num_bits.ValueOrDie());
+}
+
 }  // namespace
 
 // TODO(thestig) Using unique_ptr with ReleaseDeleter is still not ideal.
@@ -3606,13 +3612,10 @@ FX_BOOL CPDF_DataAvail::CheckHintTables(IFX_DownloadHints* pHints) {
   nonstd::unique_ptr<CPDF_HintTables> pHintTables(
       new CPDF_HintTables(this, pDict));
   CPDF_Stream* pHintStream = (CPDF_Stream*)ParseIndirectObjectAt(szHSStart, 0);
-  FX_BOOL bLoaded = FALSE;
-  if (pHintTables && pHintStream && pHintStream->GetType() == PDFOBJ_STREAM) {
-    bLoaded = pHintTables->LoadHintStream(pHintStream);
-  }
-  if (!bLoaded) {
+  if (pHintStream && pHintStream->GetType() == PDFOBJ_STREAM &&
+      pHintTables->LoadHintStream(pHintStream))
     m_pHintTables.reset(pHintTables.release());
-  }
+
   m_docStatus = PDF_DATAAVAIL_DONE;
   return TRUE;
 }
@@ -4642,12 +4645,16 @@ FX_DWORD CPDF_HintTables::GetItemLength(int index,
   return szArray[index + 1] - szArray[index];
 }
 FX_BOOL CPDF_HintTables::ReadPageHintTable(CFX_BitStream* hStream) {
-  if (!hStream)
+  if (!hStream || hStream->IsEOF())
     return FALSE;
   int nStreamOffset = ReadPrimaryHintStreamOffset();
   int nStreamLen = ReadPrimaryHintStreamLength();
   if (nStreamOffset < 0 || nStreamLen < 1)
     return FALSE;
+
+  const FX_DWORD kHeaderSize = 288;
+  if (hStream->BitsRemaining() < kHeaderSize)
+    return FALSE;
   // Item 1: The least number of objects in a page.
   FX_DWORD dwObjLeastNum = hStream->GetBits(32);
   // Item 2: The location of the first page's page object.
@@ -4690,6 +4697,11 @@ FX_BOOL CPDF_HintTables::ReadPageHintTable(CFX_BitStream* hStream) {
   int nPages = pPageNum ? pPageNum->GetInteger() : 0;
   if (nPages < 1)
     return FALSE;
+
+  FX_SAFE_DWORD required_bits = dwDeltaObjectsBits;
+  required_bits *= pdfium::base::checked_cast<FX_DWORD>(nPages);
+  if (!CanReadFromBitStream(hStream, required_bits))
+    return FALSE;
   for (int i = 0; i < nPages; ++i) {
     FX_SAFE_DWORD safeDeltaObj = hStream->GetBits(dwDeltaObjectsBits);
     safeDeltaObj += dwObjLeastNum;
@@ -4698,6 +4710,11 @@ FX_BOOL CPDF_HintTables::ReadPageHintTable(CFX_BitStream* hStream) {
     m_dwDeltaNObjsArray.Add(safeDeltaObj.ValueOrDie());
   }
   hStream->ByteAlign();
+
+  required_bits = dwDeltaPageLenBits;
+  required_bits *= pdfium::base::checked_cast<FX_DWORD>(nPages);
+  if (!CanReadFromBitStream(hStream, required_bits))
+    return FALSE;
   CFX_DWordArray dwPageLenArray;
   for (int i = 0; i < nPages; ++i) {
     FX_SAFE_DWORD safePageLen = hStream->GetBits(dwDeltaPageLenBits);
@@ -4737,41 +4754,64 @@ FX_BOOL CPDF_HintTables::ReadPageHintTable(CFX_BitStream* hStream) {
                             dwPageLenArray[nPages - 1]);
   }
   hStream->ByteAlign();
+
   // number of shared objects
+  required_bits = dwSharedObjBits;
+  required_bits *= pdfium::base::checked_cast<FX_DWORD>(nPages);
+  if (!CanReadFromBitStream(hStream, required_bits))
+    return FALSE;
   for (int i = 0; i < nPages; i++) {
     m_dwNSharedObjsArray.Add(hStream->GetBits(dwSharedObjBits));
   }
   hStream->ByteAlign();
+
   // array of identifier, sizes = nshared_objects
   for (int i = 0; i < nPages; i++) {
+    required_bits = dwSharedIdBits;
+    required_bits *= m_dwNSharedObjsArray[i];
+    if (!CanReadFromBitStream(hStream, required_bits))
+      return FALSE;
     for (int j = 0; j < m_dwNSharedObjsArray[i]; j++) {
       m_dwIdentifierArray.Add(hStream->GetBits(dwSharedIdBits));
     }
   }
   hStream->ByteAlign();
+
   for (int i = 0; i < nPages; i++) {
     FX_SAFE_DWORD safeSize = m_dwNSharedObjsArray[i];
     safeSize *= dwSharedNumeratorBits;
-    if (!safeSize.IsValid())
+    if (!CanReadFromBitStream(hStream, safeSize))
       return FALSE;
     hStream->SkipBits(safeSize.ValueOrDie());
   }
   hStream->ByteAlign();
+
   FX_SAFE_DWORD safeTotalPageLen = pdfium::base::checked_cast<FX_DWORD>(nPages);
   safeTotalPageLen *= dwDeltaPageLenBits;
-  if (!safeTotalPageLen.IsValid())
+  if (!CanReadFromBitStream(hStream, safeTotalPageLen))
     return FALSE;
   hStream->SkipBits(safeTotalPageLen.ValueOrDie());
   hStream->ByteAlign();
   return TRUE;
 }
-FX_BOOL CPDF_HintTables::ReadSharedObjHintTable(CFX_BitStream* hStream) {
-  if (!hStream)
+FX_BOOL CPDF_HintTables::ReadSharedObjHintTable(CFX_BitStream* hStream,
+                                                FX_DWORD offset) {
+  if (!hStream || hStream->IsEOF())
     return FALSE;
   int nStreamOffset = ReadPrimaryHintStreamOffset();
   int nStreamLen = ReadPrimaryHintStreamLength();
   if (nStreamOffset < 0 || nStreamLen < 1)
     return FALSE;
+
+  FX_SAFE_DWORD bit_offset = offset;
+  bit_offset *= 8;
+  if (!bit_offset.IsValid() || hStream->GetPos() > bit_offset.ValueOrDie())
+    return FALSE;
+  hStream->SkipBits(bit_offset.ValueOrDie() - hStream->GetPos());
+
+  const FX_DWORD kHeaderSize = 192;
+  if (hStream->BitsRemaining() < kHeaderSize)
+    return FALSE;
   // Item 1: The object number of the first object in the shared objects
   // section.
   FX_DWORD dwFirstSharedObjNum = hStream->GetBits(32);
@@ -4799,6 +4839,11 @@ FX_BOOL CPDF_HintTables::ReadSharedObjHintTable(CFX_BitStream* hStream) {
     return FALSE;
   FX_DWORD dwPrevObjLen = 0;
   FX_DWORD dwCurObjLen = 0;
+  FX_SAFE_DWORD required_bits = dwSharedObjTotal;
+  required_bits *= dwDeltaGroupLen;
+  if (!CanReadFromBitStream(hStream, required_bits))
+    return FALSE;
+
   for (int i = 0; i < dwSharedObjTotal; ++i) {
     dwPrevObjLen = dwCurObjLen;
     FX_SAFE_DWORD safeObjLen = hStream->GetBits(dwDeltaGroupLen);
@@ -4836,6 +4881,8 @@ FX_BOOL CPDF_HintTables::ReadSharedObjHintTable(CFX_BitStream* hStream) {
     m_szSharedObjOffsetArray.Add(safeLoc.ValueOrDie());
   }
   hStream->ByteAlign();
+  if (hStream->BitsRemaining() < dwSharedObjTotal)
+    return FALSE;
   hStream->SkipBits(dwSharedObjTotal);
   hStream->ByteAlign();
   return TRUE;
@@ -4897,6 +4944,8 @@ int32_t CPDF_HintTables::CheckPage(int index, IFX_DownloadHints* pHints) {
   FX_DWORD dwObjNum = 0;
   for (int j = 0; j < m_dwNSharedObjsArray[index]; ++j) {
     dwIndex = m_dwIdentifierArray[offset + j];
+    if (dwIndex >= m_dwSharedObjNumArray.GetSize())
+      return IPDF_DataAvail::DataNotAvailable;
     dwObjNum = m_dwSharedObjNumArray[dwIndex];
     if (dwObjNum >= nFirstPageObjNum &&
         dwObjNum < nFirstPageObjNum + m_nFirstPageSharedObjs) {
@@ -4918,6 +4967,7 @@ FX_BOOL CPDF_HintTables::LoadHintStream(CPDF_Stream* pHintStream) {
   CPDF_Object* pOffset = pDict ? pDict->GetElement(FX_BSTRC("S")) : nullptr;
   if (!pOffset || pOffset->GetType() != PDFOBJ_NUMBER)
     return FALSE;
+  int shared_hint_table_offset = pOffset->GetInteger();
   CPDF_StreamAcc acc;
   acc.LoadAllData(pHintStream);
   FX_DWORD size = acc.GetSize();
@@ -4925,13 +4975,15 @@ FX_BOOL CPDF_HintTables::LoadHintStream(CPDF_Stream* pHintStream) {
   // The header section of shared object hint table is 24 bytes.
   // Hint table has at least 60 bytes.
   const FX_DWORD MIN_STREAM_LEN = 60;
-  if (size < MIN_STREAM_LEN || size < pOffset->GetInteger() ||
-      !pOffset->GetInteger()) {
+  if (size < MIN_STREAM_LEN || shared_hint_table_offset <= 0 ||
+      size < shared_hint_table_offset) {
     return FALSE;
   }
   CFX_BitStream bs;
   bs.Init(acc.GetData(), size);
-  return ReadPageHintTable(&bs) && ReadSharedObjHintTable(&bs);
+  return ReadPageHintTable(&bs) &&
+         ReadSharedObjHintTable(&bs, pdfium::base::checked_cast<FX_DWORD>(
+                                         shared_hint_table_offset));
 }
 int CPDF_HintTables::ReadPrimaryHintStreamOffset() const {
   if (!m_pLinearizedDict)
diff --git a/core/src/fpdfapi/fpdf_parser/parser_int.h b/core/src/fpdfapi/fpdf_parser/parser_int.h
index fba6663238..c7f240384f 100644
--- a/core/src/fpdfapi/fpdf_parser/parser_int.h
+++ b/core/src/fpdfapi/fpdf_parser/parser_int.h
@@ -34,7 +34,7 @@ class CPDF_HintTables {
 
  protected:
   FX_BOOL ReadPageHintTable(CFX_BitStream* hStream);
-  FX_BOOL ReadSharedObjHintTable(CFX_BitStream* hStream);
+  FX_BOOL ReadSharedObjHintTable(CFX_BitStream* hStream, FX_DWORD offset);
   FX_DWORD GetItemLength(int index, const CFX_FileSizeArray& szArray);
 
  private: