Improve hint table validation checks.

Check required hint table dictionary entries and make sure they:
- Exist.
- Are of the right type.

Along the way:
- Fix FX_atonum() to not have a non-const pass-by-ref param.
- Simplify code in CPDF_StreamContentParser.
- Make CPDF_Number::IsInteger() a const method.

BUG=610555

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

3 files changed, 35 insertions, 23 deletions

diff --git a/core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp b/core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp
index 5a7a7be544..cc5b529b45 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp
@@ -6,6 +6,8 @@
 
 #include "core/fpdfapi/fpdf_parser/include/cpdf_data_avail.h"
 
+#include <algorithm>
+
 #include "core/fpdfapi/fpdf_parser/cpdf_hint_tables.h"
 #include "core/fpdfapi/fpdf_parser/fpdf_parser_utility.h"
 #include "core/fpdfapi/fpdf_parser/include/cpdf_array.h"
@@ -723,37 +725,48 @@ FX_BOOL CPDF_DataAvail::CheckHintTables(DownloadHints* pHints) {
     return FALSE;
   }
 
-  if (!pDict->KeyExist("H") || !pDict->KeyExist("O") || !pDict->KeyExist("N")) {
+  // The actual value is not required here, but validate its existence and type.
+  CPDF_Number* pFirstPage = ToNumber(pDict->GetDirectObjectBy("O"));
+  if (!pFirstPage || !pFirstPage->IsInteger()) {
+    m_docStatus = PDF_DATAAVAIL_ERROR;
+    return FALSE;
+  }
+
+  CPDF_Number* pPageCount = ToNumber(pDict->GetDirectObjectBy("N"));
+  if (!pPageCount || !pPageCount->IsInteger()) {
     m_docStatus = PDF_DATAAVAIL_ERROR;
     return FALSE;
   }
 
-  int nPageCount = pDict->GetDirectObjectBy("N")->GetInteger();
+  int nPageCount = pPageCount->GetInteger();
   if (nPageCount <= 1) {
     m_docStatus = PDF_DATAAVAIL_DONE;
     return TRUE;
   }
 
   CPDF_Array* pHintStreamRange = pDict->GetArrayBy("H");
-  if (!pHintStreamRange) {
+  size_t nHintStreamSize = pHintStreamRange ? pHintStreamRange->GetCount() : 0;
+  if (nHintStreamSize != 2 && nHintStreamSize != 4) {
     m_docStatus = PDF_DATAAVAIL_ERROR;
     return FALSE;
   }
 
-  FX_FILESIZE szHSStart =
-      pHintStreamRange->GetDirectObjectAt(0)
-          ? pHintStreamRange->GetDirectObjectAt(0)->GetInteger()
-          : 0;
-  FX_FILESIZE szHSLength =
-      pHintStreamRange->GetDirectObjectAt(1)
-          ? pHintStreamRange->GetDirectObjectAt(1)->GetInteger()
-          : 0;
-  if (szHSStart < 0 || szHSLength <= 0) {
+  for (const CPDF_Object* pArrayObject : *pHintStreamRange) {
+    const CPDF_Number* pNumber = ToNumber(pArrayObject->GetDirect());
+    if (!pNumber || !pNumber->IsInteger()) {
+      m_docStatus = PDF_DATAAVAIL_ERROR;
+      return FALSE;
+    }
+  }
+
+  FX_FILESIZE szHintStart = pHintStreamRange->GetIntegerAt(0);
+  FX_FILESIZE szHintLength = pHintStreamRange->GetIntegerAt(1);
+  if (szHintStart < 0 || szHintLength <= 0) {
     m_docStatus = PDF_DATAAVAIL_ERROR;
     return FALSE;
   }
 
-  if (!IsDataAvail(szHSStart, szHSLength, pHints))
+  if (!IsDataAvail(szHintStart, szHintLength, pHints))
     return FALSE;
 
   m_syntaxParser.InitParser(m_pFileRead, m_dwHeaderOffset);
@@ -761,7 +774,7 @@ FX_BOOL CPDF_DataAvail::CheckHintTables(DownloadHints* pHints) {
   std::unique_ptr<CPDF_HintTables> pHintTables(
       new CPDF_HintTables(this, pDict));
   std::unique_ptr<CPDF_Object, ReleaseDeleter<CPDF_Object>> pHintStream(
-      ParseIndirectObjectAt(szHSStart, 0));
+      ParseIndirectObjectAt(szHintStart, 0));
   CPDF_Stream* pStream = ToStream(pHintStream.get());
   if (pStream && pHintTables->LoadHintStream(pStream))
     m_pHintTables = std::move(pHintTables);
diff --git a/core/fpdfapi/fpdf_parser/cpdf_number.cpp b/core/fpdfapi/fpdf_parser/cpdf_number.cpp
index a6bace57c9..5012e308ce 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_number.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_number.cpp
@@ -6,15 +6,14 @@
 
 #include "core/fpdfapi/fpdf_parser/include/cpdf_number.h"
 
-CPDF_Number::CPDF_Number() : m_bInteger(TRUE), m_Integer(0) {}
+CPDF_Number::CPDF_Number() : m_bInteger(true), m_Integer(0) {}
 
-CPDF_Number::CPDF_Number(int value) : m_bInteger(TRUE), m_Integer(value) {}
+CPDF_Number::CPDF_Number(int value) : m_bInteger(true), m_Integer(value) {}
 
-CPDF_Number::CPDF_Number(FX_FLOAT value) : m_bInteger(FALSE), m_Float(value) {}
+CPDF_Number::CPDF_Number(FX_FLOAT value) : m_bInteger(false), m_Float(value) {}
 
-CPDF_Number::CPDF_Number(const CFX_ByteStringC& str) {
-  FX_atonum(str, m_bInteger, &m_Integer);
-}
+CPDF_Number::CPDF_Number(const CFX_ByteStringC& str)
+    : m_bInteger(FX_atonum(str, &m_Integer)) {}
 
 CPDF_Number::~CPDF_Number() {}
 
@@ -47,7 +46,7 @@ const CPDF_Number* CPDF_Number::AsNumber() const {
 }
 
 void CPDF_Number::SetString(const CFX_ByteString& str) {
-  FX_atonum(str.AsStringC(), m_bInteger, &m_Integer);
+  m_bInteger = FX_atonum(str.AsStringC(), &m_Integer);
 }
 
 CFX_ByteString CPDF_Number::GetString() const {
diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_number.h b/core/fpdfapi/fpdf_parser/include/cpdf_number.h
index 7b93a992b1..eaa4009543 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_number.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_number.h
@@ -29,12 +29,12 @@ class CPDF_Number : public CPDF_Object {
   CPDF_Number* AsNumber() override;
   const CPDF_Number* AsNumber() const override;
 
-  FX_BOOL IsInteger() { return m_bInteger; }
+  bool IsInteger() const { return m_bInteger; }
 
  protected:
   ~CPDF_Number() override;
 
-  FX_BOOL m_bInteger;
+  bool m_bInteger;
   union {
     int m_Integer;
     FX_FLOAT m_Float;