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
diff --git a/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp b/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp
index 802f424..0d16994 100644
--- a/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp
+++ b/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp
@@ -274,35 +274,36 @@
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), ¶m.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 @@
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 29fd6fb..08cfd04 100644
--- a/core/fpdfapi/fpdf_page/pageint.h
+++ b/core/fpdfapi/fpdf_page/pageint.h
@@ -87,7 +87,7 @@
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 5a7a7be..cc5b529 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 @@
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;
}
- int nPageCount = pDict->GetDirectObjectBy("N")->GetInteger();
+ CPDF_Number* pPageCount = ToNumber(pDict->GetDirectObjectBy("N"));
+ if (!pPageCount || !pPageCount->IsInteger()) {
+ m_docStatus = PDF_DATAAVAIL_ERROR;
+ return FALSE;
+ }
+
+ 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 @@
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 a6bace5..5012e30 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 @@
}
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 7b93a99..eaa4009 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_number.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_number.h
@@ -29,12 +29,12 @@
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 12d3aef..abd84a8 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 b0a89fd..c1bd82d 100644
--- a/core/fxcrt/include/fx_string.h
+++ b/core/fxcrt/include/fx_string.h
@@ -437,7 +437,7 @@
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_
