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

7 files changed, 85 insertions, 72 deletions

diff --git a/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp b/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp
index 802f424274..0d16994bbe 100644
--- a/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp
+++ b/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp
@@ -274,35 +274,36 @@ int CPDF_StreamContentParser::GetNextParamPos() {
 
 void CPDF_StreamContentParser::AddNameParam(const FX_CHAR* name, int len) {
   CFX_ByteStringC bsName(name, len);
-  int index = GetNextParamPos();
+  ContentParam& param = m_ParamBuf[GetNextParamPos()];
   if (len > 32) {
-    m_ParamBuf[index].m_Type = ContentParam::OBJECT;
-    m_ParamBuf[index].m_pObject = new CPDF_Name(PDF_NameDecode(bsName));
+    param.m_Type = ContentParam::OBJECT;
+    param.m_pObject = new CPDF_Name(PDF_NameDecode(bsName));
   } else {
-    m_ParamBuf[index].m_Type = ContentParam::NAME;
+    param.m_Type = ContentParam::NAME;
     if (bsName.Find('#') == -1) {
-      FXSYS_memcpy(m_ParamBuf[index].m_Name.m_Buffer, name, len);
-      m_ParamBuf[index].m_Name.m_Len = len;
+      FXSYS_memcpy(param.m_Name.m_Buffer, name, len);
+      param.m_Name.m_Len = len;
     } else {
       CFX_ByteString str = PDF_NameDecode(bsName);
-      FXSYS_memcpy(m_ParamBuf[index].m_Name.m_Buffer, str.c_str(),
-                   str.GetLength());
-      m_ParamBuf[index].m_Name.m_Len = str.GetLength();
+      FXSYS_memcpy(param.m_Name.m_Buffer, str.c_str(), str.GetLength());
+      param.m_Name.m_Len = str.GetLength();
     }
   }
 }
 
 void CPDF_StreamContentParser::AddNumberParam(const FX_CHAR* str, int len) {
-  int index = GetNextParamPos();
-  m_ParamBuf[index].m_Type = ContentParam::NUMBER;
-  FX_atonum(CFX_ByteStringC(str, len), m_ParamBuf[index].m_Number.m_bInteger,
-            &m_ParamBuf[index].m_Number.m_Integer);
+  ContentParam& param = m_ParamBuf[GetNextParamPos()];
+  param.m_Type = ContentParam::NUMBER;
+  param.m_Number.m_bInteger =
+      FX_atonum(CFX_ByteStringC(str, len), &param.m_Number.m_Integer);
 }
+
 void CPDF_StreamContentParser::AddObjectParam(CPDF_Object* pObj) {
-  int index = GetNextParamPos();
-  m_ParamBuf[index].m_Type = ContentParam::OBJECT;
-  m_ParamBuf[index].m_pObject = pObj;
+  ContentParam& param = m_ParamBuf[GetNextParamPos()];
+  param.m_Type = ContentParam::OBJECT;
+  param.m_pObject = pObj;
 }
+
 void CPDF_StreamContentParser::ClearAllParams() {
   uint32_t index = m_ParamStartPos;
   for (uint32_t i = 0; i < m_ParamCount; i++) {
@@ -1601,14 +1602,13 @@ void CPDF_StreamContentParser::ParsePathObject() {
         break;
       }
       case CPDF_StreamParser::Number: {
-        if (nParams == 6) {
+        if (nParams == 6)
           break;
-        }
-        FX_BOOL bInteger;
+
         int value;
-        FX_atonum(
+        bool bInteger = FX_atonum(
             CFX_ByteStringC(m_pSyntax->GetWordBuf(), m_pSyntax->GetWordSize()),
-            bInteger, &value);
+            &value);
         params[nParams++] = bInteger ? (FX_FLOAT)value : *(FX_FLOAT*)&value;
         break;
       }
diff --git a/core/fpdfapi/fpdf_page/pageint.h b/core/fpdfapi/fpdf_page/pageint.h
index 29fd6fb86e..08cfd0472a 100644
--- a/core/fpdfapi/fpdf_page/pageint.h
+++ b/core/fpdfapi/fpdf_page/pageint.h
@@ -87,7 +87,7 @@ struct ContentParam {
   Type m_Type;
   union {
     struct {
-      FX_BOOL m_bInteger;
+      bool m_bInteger;
       union {
         int m_Integer;
         FX_FLOAT m_Float;
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;
diff --git a/core/fxcrt/fx_basic_util.cpp b/core/fxcrt/fx_basic_util.cpp
index 12d3aefd1c..abd84a864f 100644
--- a/core/fxcrt/fx_basic_util.cpp
+++ b/core/fxcrt/fx_basic_util.cpp
@@ -18,33 +18,34 @@
 #include <cctype>
 #include <memory>
 
-void FX_atonum(const CFX_ByteStringC& strc, FX_BOOL& bInteger, void* pData) {
-  if (strc.Find('.') == -1) {
-    bInteger = TRUE;
-    int cc = 0;
-    pdfium::base::CheckedNumeric<int> integer = 0;
-    FX_STRSIZE len = strc.GetLength();
-    bool bNegative = false;
-    if (strc[0] == '+') {
-      cc++;
-    } else if (strc[0] == '-') {
-      bNegative = true;
-      cc++;
-    }
-    while (cc < len && std::isdigit(strc[cc])) {
-      integer = integer * 10 + FXSYS_toDecimalDigit(strc.CharAt(cc));
-      if (!integer.IsValid())
-        break;
-      cc++;
-    }
-    if (bNegative) {
-      integer = -integer;
-    }
-    *(int*)pData = integer.ValueOrDefault(0);
-  } else {
-    bInteger = FALSE;
-    *(FX_FLOAT*)pData = FX_atof(strc);
+bool FX_atonum(const CFX_ByteStringC& strc, void* pData) {
+  if (strc.Find('.') != -1) {
+    FX_FLOAT* pFloat = static_cast<FX_FLOAT*>(pData);
+    *pFloat = FX_atof(strc);
+    return false;
+  }
+
+  int cc = 0;
+  pdfium::base::CheckedNumeric<int> integer = 0;
+  bool bNegative = false;
+  if (strc[0] == '+') {
+    cc++;
+  } else if (strc[0] == '-') {
+    bNegative = true;
+    cc++;
   }
+  while (cc < strc.GetLength() && std::isdigit(strc[cc])) {
+    integer = integer * 10 + FXSYS_toDecimalDigit(strc.CharAt(cc));
+    if (!integer.IsValid())
+      break;
+    cc++;
+  }
+  if (bNegative)
+    integer = -integer;
+
+  int* pInt = static_cast<int*>(pData);
+  *pInt = integer.ValueOrDefault(0);
+  return true;
 }
 
 static const FX_FLOAT fraction_scales[] = {
diff --git a/core/fxcrt/include/fx_string.h b/core/fxcrt/include/fx_string.h
index b0a89fd06c..c1bd82dd74 100644
--- a/core/fxcrt/include/fx_string.h
+++ b/core/fxcrt/include/fx_string.h
@@ -437,7 +437,7 @@ FX_FLOAT FX_atof(const CFX_ByteStringC& str);
 inline FX_FLOAT FX_atof(const CFX_WideStringC& wsStr) {
   return FX_atof(FX_UTF8Encode(wsStr.c_str(), wsStr.GetLength()).c_str());
 }
-void FX_atonum(const CFX_ByteStringC& str, FX_BOOL& bInteger, void* pData);
+bool FX_atonum(const CFX_ByteStringC& str, void* pData);
 FX_STRSIZE FX_ftoa(FX_FLOAT f, FX_CHAR* buf);
 
 #endif  // CORE_FXCRT_INCLUDE_FX_STRING_H_