Endless loop in CFGAS_FormatString::FormatStrNum().
Comparison didn't take sign into account, nor the possibility
of CFGAS_Decimal overflowing to 0 (fixing that should be a
separate issue).
Fix another obvious overflow while computing threshold.
Bug: pdfium:1244
Change-Id: Idbbfd311a60b57f83b18ee2fa8af45eeafa877f8
Reviewed-on: https://pdfium-review.googlesource.com/c/50894
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/xfa/fgas/crt/cfgas_decimal.h b/xfa/fgas/crt/cfgas_decimal.h
index ddfd8e8..4751c5b 100644
--- a/xfa/fgas/crt/cfgas_decimal.h
+++ b/xfa/fgas/crt/cfgas_decimal.h
@@ -25,6 +25,7 @@
CFGAS_Decimal operator*(const CFGAS_Decimal& val) const;
CFGAS_Decimal operator/(const CFGAS_Decimal& val) const;
+ bool IsNotZero() const { return m_uHi || m_uMid || m_uLo; }
uint8_t GetScale() const { return m_uScale; }
void SetScale(uint8_t newScale);
void SetNegate();
@@ -35,7 +36,6 @@
uint32_t lo,
bool neg,
uint8_t scale);
- bool IsNotZero() const { return m_uHi || m_uMid || m_uLo; }
uint32_t m_uHi = 0;
uint32_t m_uMid = 0;
diff --git a/xfa/fgas/crt/cfgas_formatstring.cpp b/xfa/fgas/crt/cfgas_formatstring.cpp
index 7cc6d93..45e0445 100644
--- a/xfa/fgas/crt/cfgas_formatstring.cpp
+++ b/xfa/fgas/crt/cfgas_formatstring.cpp
@@ -11,6 +11,7 @@
#include <vector>
#include "core/fxcrt/fx_extension.h"
+#include "core/fxcrt/fx_safe_types.h"
#include "xfa/fgas/crt/cfgas_decimal.h"
#define FX_LOCALECATEGORY_DateHash 0xbde9abde
@@ -1896,25 +1897,30 @@
}
}
- int threshold = 1;
+ FX_SAFE_UINT32 threshold = 1;
while (fixed_count > 1) {
threshold *= 10;
fixed_count--;
}
- if (decimal.ToDouble() != 0.0) {
- if (decimal.ToDouble() < threshold) {
- decimal = decimal * CFGAS_Decimal(10);
- exponent = -1;
- while (decimal.ToDouble() < threshold) {
- decimal = decimal * CFGAS_Decimal(10);
- exponent -= 1;
- }
- } else if (decimal.ToDouble() > threshold) {
- threshold *= 10;
- while (decimal.ToDouble() > threshold) {
- decimal = decimal / CFGAS_Decimal(10);
- exponent += 1;
- }
+ if (!threshold.IsValid())
+ return false;
+
+ bool bAdjusted = false;
+ while (decimal.IsNotZero() &&
+ fabs(decimal.ToDouble()) < threshold.ValueOrDie()) {
+ decimal = decimal * CFGAS_Decimal(10);
+ --exponent;
+ bAdjusted = true;
+ }
+ if (!bAdjusted) {
+ threshold *= 10;
+ if (!threshold.IsValid())
+ return false;
+
+ while (decimal.IsNotZero() &&
+ fabs(decimal.ToDouble()) > threshold.ValueOrDie()) {
+ decimal = decimal / CFGAS_Decimal(10);
+ ++exponent;
}
}
}
@@ -2077,14 +2083,12 @@
}
if (bNeg)
*wsOutput = pLocale->GetMinusSymbol() + *wsOutput;
-
- return false;
+ return true;
}
if (dot_index_f ==
pdfium::base::checked_cast<int32_t>(wsNumFormat.GetLength())) {
if (!bAddNeg && bNeg)
*wsOutput = pLocale->GetMinusSymbol() + *wsOutput;
-
return true;
}
diff --git a/xfa/fgas/crt/cfgas_formatstring_unittest.cpp b/xfa/fgas/crt/cfgas_formatstring_unittest.cpp
index 8bc77a5..a3b7bd4 100644
--- a/xfa/fgas/crt/cfgas_formatstring_unittest.cpp
+++ b/xfa/fgas/crt/cfgas_formatstring_unittest.cpp
@@ -303,12 +303,14 @@
}
TEST_F(CFGAS_FormatStringTest, NumParse) {
- struct {
+ struct TestCase {
const wchar_t* locale;
const wchar_t* input;
const wchar_t* pattern;
const wchar_t* output;
- } tests[] = {
+ };
+
+ static const TestCase tests[] = {
// {L"en", L"€100.00", L"num(en_GB){$z,zz9.99}", L"100"},
// {L"en", L"1050", L"99V99", L"10.50"},
// {L"en", L"3125", L"99V99", L"31.25"},
@@ -415,22 +417,24 @@
{L"en", L"123.545,4", L"zzz.zzz,z", L"123.5454"},
};
- for (size_t i = 0; i < FX_ArraySize(tests); ++i) {
+ for (const auto& test : tests) {
WideString result;
- EXPECT_TRUE(fmt(tests[i].locale)
- ->ParseNum(tests[i].input, tests[i].pattern, &result))
- << " TEST: " << i;
- EXPECT_STREQ(tests[i].output, result.c_str()) << " TEST: " << i;
+ EXPECT_TRUE(fmt(test.locale)->ParseNum(test.input, test.pattern, &result))
+ << " TEST: " << test.input << ", " << test.pattern;
+ EXPECT_STREQ(test.output, result.c_str())
+ << " TEST: " << test.input << ", " << test.pattern;
}
}
TEST_F(CFGAS_FormatStringTest, NumFormat) {
- struct {
+ struct TestCase {
const wchar_t* locale;
const wchar_t* input;
const wchar_t* pattern;
const wchar_t* output;
- } tests[] = {
+ };
+
+ static const TestCase tests[] = {
{L"en", L"1.234", L"zz9.zzz", L"1.234"},
{L"en", L"1", L"num{z 'text'}", L"1 text"},
{L"en", L"1", L"num{'text' z}", L"text 1"},
@@ -518,14 +522,21 @@
{L"en", L"1", L"9.C", L"1"},
{L"en", L"1", L"9.d", L"1"},
{L"en", L"1", L"9.D", L"1"},
+ // https://crbug.com/pdfium/1244
+ {L"en", L"1", L"E", L"1"},
+ {L"en", L"0", L"E", L"0"},
+ {L"en", L"-1", L"E", L"-1"},
+ {L"en", L"900000000000000000000", L"E", L"900,000,000,000,000,000,000"},
+ // TODO(tsepez): next one seems wrong
+ // {L"en", L".000000000000000000009", L"E", L"9"},
};
- for (size_t i = 0; i < FX_ArraySize(tests); ++i) {
+ for (const auto& test : tests) {
WideString result;
- EXPECT_TRUE(fmt(tests[i].locale)
- ->FormatNum(tests[i].input, tests[i].pattern, &result))
- << " TEST: " << i;
- EXPECT_STREQ(tests[i].output, result.c_str()) << " TEST: " << i;
+ EXPECT_TRUE(fmt(test.locale)->FormatNum(test.input, test.pattern, &result))
+ << " TEST: " << test.input << ", " << test.pattern;
+ EXPECT_STREQ(test.output, result.c_str())
+ << " TEST: " << test.input << ", " << test.pattern;
}
}
