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>

xfa/fgas/crt/cfgas_decimal.h

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;

xfa/fgas/crt/cfgas_formatstring.cpp

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;
   }
 

xfa/fgas/crt/cfgas_formatstring_unittest.cpp

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;
   }
 }