Fix unsafe buffer usage in (non-XFA) JS bindings.
-- convert to std::array<>, or
-- avoid indexing with range-based loop, or
-- mark two routines UNSAFE.
Change-Id: I5f2ed1819d139c82e69e89c7502ccfa931d1cdf2
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/118074
Reviewed-by: Thomas Sepez <tsepez@google.com>
Commit-Queue: Thomas Sepez <tsepez@google.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/fxjs/cjs_globalarrays.cpp b/fxjs/cjs_globalarrays.cpp
index 26729e1..dfd9bc2 100644
--- a/fxjs/cjs_globalarrays.cpp
+++ b/fxjs/cjs_globalarrays.cpp
@@ -14,14 +14,13 @@
#define GLOBAL_ARRAY(rt, name, ...) \
{ \
- static const wchar_t* const values[] = {__VA_ARGS__}; \
+ static const wchar_t* const kValues[] = {__VA_ARGS__}; \
v8::Local<v8::Array> array = (rt)->NewArray(); \
v8::Local<v8::Context> ctx = (rt)->GetIsolate()->GetCurrentContext(); \
- for (size_t i = 0; i < std::size(values); ++i) { \
- array \
- ->Set(ctx, pdfium::checked_cast<uint32_t>(i), \
- (rt)->NewString(values[i])) \
- .FromJust(); \
+ uint32_t i = 0; \
+ for (const auto* value : kValues) { \
+ array->Set(ctx, i, (rt)->NewString(value)).FromJust(); \
+ ++i; \
} \
(rt)->SetConstArray((name), array); \
(rt)->DefineGlobalConst( \
diff --git a/fxjs/cjs_publicmethods.cpp b/fxjs/cjs_publicmethods.cpp
index aeaf0ad..9d2b683 100644
--- a/fxjs/cjs_publicmethods.cpp
+++ b/fxjs/cjs_publicmethods.cpp
@@ -9,6 +9,7 @@
#include <math.h>
#include <algorithm>
+#include <array>
#include <iomanip>
#include <iterator>
#include <limits>
@@ -23,6 +24,7 @@
#include "core/fpdfdoc/cpdf_formcontrol.h"
#include "core/fpdfdoc/cpdf_interactiveform.h"
#include "core/fxcrt/check.h"
+#include "core/fxcrt/compiler_specific.h"
#include "core/fxcrt/fx_extension.h"
#include "core/fxcrt/fx_string_wrappers.h"
#include "core/fxcrt/numerics/safe_conversions.h"
@@ -73,13 +75,13 @@
constexpr double kDoubleCorrect = 0.000000000000001;
#endif
-constexpr const char* kDateFormats[] = {
- "m/d", "m/d/yy", "mm/dd/yy", "mm/yy", "d-mmm",
- "d-mmm-yy", "dd-mmm-yy", "yy-mm-dd", "mmm-yy", "mmmm-yy",
- "mmm d, yyyy", "mmmm d, yyyy", "m/d/yy h:MM tt", "m/d/yy HH:MM"};
+constexpr std::array<const char*, 14> kDateFormats = {
+ {"m/d", "m/d/yy", "mm/dd/yy", "mm/yy", "d-mmm", "d-mmm-yy", "dd-mmm-yy",
+ "yy-mm-dd", "mmm-yy", "mmmm-yy", "mmm d, yyyy", "mmmm d, yyyy",
+ "m/d/yy h:MM tt", "m/d/yy HH:MM"}};
-constexpr const char* kTimeFormats[] = {"HH:MM", "h:MM tt", "HH:MM:ss",
- "h:MM:ss tt"};
+constexpr std::array<const char*, 4> kTimeFormats = {
+ {"HH:MM", "h:MM tt", "HH:MM:ss", "h:MM:ss tt"}};
template <typename T>
T StrTrim(const T& str) {
@@ -263,30 +265,36 @@
bool bDot = false;
bool bKXJS = false;
- wchar_t c;
- while ((c = *p) != L'\0') {
- if (IsDigitSeparatorOrDecimalMark(c)) {
- if (bDot)
- return false;
- bDot = true;
- } else if (c == L'-' || c == L'+') {
- if (p != pTrim)
- return false;
- } else if (c == L'e' || c == L'E') {
- if (bKXJS)
- return false;
+ // TODO(tsepez): fix UNSAFE usage.
+ UNSAFE_BUFFERS({
+ wchar_t c;
+ while ((c = *p) != L'\0') {
+ if (IsDigitSeparatorOrDecimalMark(c)) {
+ if (bDot) {
+ return false;
+ }
+ bDot = true;
+ } else if (c == L'-' || c == L'+') {
+ if (p != pTrim) {
+ return false;
+ }
+ } else if (c == L'e' || c == L'E') {
+ if (bKXJS) {
+ return false;
+ }
+ p++;
+ c = *p;
+ if (c != L'+' && c != L'-') {
+ return false;
+ }
+ bKXJS = true;
+ } else if (!FXSYS_IsDecimalDigit(c)) {
+ return false;
+ }
p++;
- c = *p;
- if (c != L'+' && c != L'-')
- return false;
- bKXJS = true;
- } else if (!FXSYS_IsDecimalDigit(c)) {
- return false;
}
- p++;
- }
-
+ });
return true;
}
@@ -322,22 +330,27 @@
int nIndex = 0;
v8::Local<v8::Array> StrArray = pRuntime->NewArray();
- while (*p) {
- const char* pTemp = strchr(p, ',');
- if (!pTemp) {
+
+ // TODO(tsepez): fix UNSAFE usage.
+ UNSAFE_BUFFERS({
+ while (*p) {
+ const char* pTemp = strchr(p, ',');
+ if (!pTemp) {
+ pRuntime->PutArrayElement(
+ StrArray, nIndex,
+ pRuntime->NewString(StrTrim(ByteString(p)).AsStringView()));
+ break;
+ }
+
pRuntime->PutArrayElement(
StrArray, nIndex,
- pRuntime->NewString(StrTrim(ByteString(p)).AsStringView()));
- break;
+ pRuntime->NewString(
+ StrTrim(ByteString(p, pTemp - p)).AsStringView()));
+
+ nIndex++;
+ p = ++pTemp;
}
-
- pRuntime->PutArrayElement(
- StrArray, nIndex,
- pRuntime->NewString(StrTrim(ByteString(p, pTemp - p)).AsStringView()));
-
- nIndex++;
- p = ++pTemp;
- }
+ });
return StrArray;
}
@@ -352,7 +365,7 @@
int nMin = FX_GetMinFromTime(dt);
int nSec = FX_GetSecFromTime(dt);
- int number[3];
+ std::array<int, 3> number;
size_t nSkip = 0;
size_t nLen = value.GetLength();
diff --git a/fxjs/cjs_publicmethods_unittest.cpp b/fxjs/cjs_publicmethods_unittest.cpp
index a5b9314..4ebf1cc 100644
--- a/fxjs/cjs_publicmethods_unittest.cpp
+++ b/fxjs/cjs_publicmethods_unittest.cpp
@@ -44,9 +44,7 @@
{L"0123", true},
{L"9876123", true},
};
- for (size_t i = 0; i < std::size(test_data); ++i) {
- EXPECT_EQ(test_data[i].expected,
- CJS_PublicMethods::IsNumber(test_data[i].input))
- << "for case " << i;
+ for (const auto& element : test_data) {
+ EXPECT_EQ(element.expected, CJS_PublicMethods::IsNumber(element.input));
}
}
diff --git a/fxjs/cjs_util.cpp b/fxjs/cjs_util.cpp
index a404ee4..9362252 100644
--- a/fxjs/cjs_util.cpp
+++ b/fxjs/cjs_util.cpp
@@ -47,7 +47,7 @@
int value;
};
-const TbConvert TbConvertTable[] = {
+const TbConvert kTbConvertTable[] = {
{L"mmmm", L"%B"}, {L"mmm", L"%b"}, {L"mm", L"%m"}, {L"dddd", L"%A"},
{L"ddd", L"%a"}, {L"dd", L"%d"}, {L"yyyy", L"%Y"}, {L"yy", L"%y"},
{L"HH", L"%H"}, {L"hh", L"%I"}, {L"MM", L"%M"}, {L"ss", L"%S"},
@@ -217,40 +217,40 @@
cFormat.erase(std::remove(cFormat.begin(), cFormat.end(), '%'),
cFormat.end());
- for (size_t i = 0; i < std::size(TbConvertTable); ++i) {
+ for (const auto& conversion : kTbConvertTable) {
size_t nFound = 0;
while (true) {
- nFound = cFormat.find(TbConvertTable[i].lpszJSMark, nFound);
- if (nFound == std::wstring::npos)
+ nFound = cFormat.find(conversion.lpszJSMark, nFound);
+ if (nFound == std::wstring::npos) {
break;
-
- cFormat.replace(nFound, wcslen(TbConvertTable[i].lpszJSMark),
- TbConvertTable[i].lpszCppMark);
+ }
+ cFormat.replace(nFound, wcslen(conversion.lpszJSMark),
+ conversion.lpszCppMark);
}
}
if (year < 0)
return CJS_Result::Failure(JSMessage::kValueError);
- const TbConvertAdditional cTableAd[] = {
+ const TbConvertAdditional table_additional[] = {
{L'm', month}, {L'd', day},
{L'H', hour}, {L'h', hour > 12 ? hour - 12 : hour},
{L'M', min}, {L's', sec},
};
- for (size_t i = 0; i < std::size(cTableAd); ++i) {
+ for (const auto& conversion : table_additional) {
size_t nFound = 0;
while (true) {
- nFound = cFormat.find(cTableAd[i].js_mark, nFound);
- if (nFound == std::wstring::npos)
+ nFound = cFormat.find(conversion.js_mark, nFound);
+ if (nFound == std::wstring::npos) {
break;
-
+ }
if (nFound != 0 && cFormat[nFound - 1] == L'%') {
++nFound;
continue;
}
cFormat.replace(nFound, 1,
- WideString::FormatInteger(cTableAd[i].value).c_str());
+ WideString::FormatInteger(conversion.value).c_str());
}
}
diff --git a/fxjs/cjs_util_unittest.cpp b/fxjs/cjs_util_unittest.cpp
index b925268..34385d8 100644
--- a/fxjs/cjs_util_unittest.cpp
+++ b/fxjs/cjs_util_unittest.cpp
@@ -106,9 +106,9 @@
{L"%10s", CJS_Util::DataType::kString},
};
- for (size_t i = 0; i < std::size(cases); i++) {
- WideString input(cases[i].input_string);
- EXPECT_EQ(cases[i].expected, CJS_Util::ParseDataType(&input))
- << cases[i].input_string;
+ for (const auto& item : cases) {
+ WideString input(item.input_string);
+ EXPECT_EQ(item.expected, CJS_Util::ParseDataType(&input))
+ << item.input_string;
}
}
diff --git a/fxjs/fx_date_helpers.cpp b/fxjs/fx_date_helpers.cpp
index 9601e96..96a3d35 100644
--- a/fxjs/fx_date_helpers.cpp
+++ b/fxjs/fx_date_helpers.cpp
@@ -10,6 +10,7 @@
#include <time.h>
#include <wctype.h>
+#include <array>
#include <iterator>
#include "build/build_config.h"
@@ -20,10 +21,11 @@
namespace fxjs {
namespace {
-constexpr uint16_t daysMonth[12] = {0, 31, 59, 90, 120, 151,
- 181, 212, 243, 273, 304, 334};
-constexpr uint16_t leapDaysMonth[12] = {0, 31, 60, 91, 121, 152,
- 182, 213, 244, 274, 305, 335};
+constexpr std::array<uint16_t, 12> kDaysMonth = {
+ {0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334}};
+
+constexpr std::array<uint16_t, 12> kLeapDaysMonth = {
+ {0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335}};
double Mod(double x, double y) {
double r = fmod(x, y);
@@ -74,8 +76,8 @@
}
double TimeFromYearMonth(int y, int m) {
- const uint16_t* pMonth = IsLeapYear(y) ? leapDaysMonth : daysMonth;
- return TimeFromYear(y) + ((double)pMonth[m]) * 86400000;
+ const uint16_t month = IsLeapYear(y) ? kLeapDaysMonth[m] : kDaysMonth[m];
+ return TimeFromYear(y) + static_cast<double>(month) * 86400000;
}
int Day(double t) {
@@ -113,8 +115,8 @@
--day;
// Check for February onwards.
- static constexpr int kCumulativeDaysInMonths[] = {
- 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365};
+ static constexpr std::array<int, 11> kCumulativeDaysInMonths = {
+ {59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365}};
for (size_t i = 0; i < std::size(kCumulativeDaysInMonths); ++i) {
if (day < kCumulativeDaysInMonths[i])
return static_cast<int>(i) + 1;
@@ -168,12 +170,13 @@
} // namespace
-const char* const kMonths[12] = {"Jan", "Feb", "Mar", "Apr", "May", "Jun",
- "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"};
+const std::array<const char*, 12> kMonths = {{"Jan", "Feb", "Mar", "Apr", "May",
+ "Jun", "Jul", "Aug", "Sep", "Oct",
+ "Nov", "Dec"}};
-const char* const kFullMonths[12] = {
- "January", "February", "March", "April", "May", "June",
- "July", "August", "September", "October", "November", "December"};
+const std::array<const char*, 12> kFullMonths = {
+ {"January", "February", "March", "April", "May", "June", "July", "August",
+ "September", "October", "November", "December"}};
static constexpr size_t KMonthAbbreviationLength = 3; // Anything in |kMonths|.
static constexpr size_t kLongestFullMonthLength = 9; // September
diff --git a/fxjs/fx_date_helpers.h b/fxjs/fx_date_helpers.h
index ca730e5..a3fc55d 100644
--- a/fxjs/fx_date_helpers.h
+++ b/fxjs/fx_date_helpers.h
@@ -9,14 +9,16 @@
#include <stddef.h>
+#include <array>
+
#include "core/fxcrt/widestring.h"
namespace fxjs {
enum class ConversionStatus { kSuccess = 0, kBadFormat, kBadDate };
-extern const char* const kMonths[12];
-extern const char* const kFullMonths[12];
+extern const std::array<const char*, 12> kMonths;
+extern const std::array<const char*, 12> kFullMonths;
double FX_GetDateTime();
int FX_GetYearFromTime(double dt);
