CVE-2018-18928: Integer Overflow or Wraparound (CWE-190)
value: International Components for Unicode (ICU) for C/C++ 63.1 has an integer overflow in number::impl::DecimalQuantity::toScientificString() in i18n/number_decimalquantity.cpp.
CWE description: The product performs a calculation that can
         produce an integer overflow or wraparound when the logic
         assumes that the resulting value will always be larger than
         the original value. This occurs when an integer value is
         incremented to a value that is too large to store in the
         associated representation. When this occurs, the value may
         become a very small or negative number.
CVSS v3 base score: 9.8
Published: 2018-11-04T20:29Z
Repository: https://github.com/unicode-org/icu
Language: C++
Files changed: icu4c/source/i18n/fmtable.cpp, icu4c/source/i18n/number_decimalquantity.cpp, icu4c/source/test/intltest/numfmtst.cpp, icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java, icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java, icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java
Commit date: 2018-10-31 13:37:09 -0700
Commit message: ICU-20246 Fixing another integer overflow in number parsing.

Diff: diff --git a/icu4c/source/i18n/fmtable.cpp b/icu4c/source/i18n/fmtable.cpp
index 45c7024fc2..8601d95f4a 100644
--- a/icu4c/source/i18n/fmtable.cpp
+++ b/icu4c/source/i18n/fmtable.cpp
@@ -732,11 +732,11 @@ CharString *Formattable::internalGetCharString(UErrorCode &status) {
       // Older ICUs called uprv_decNumberToString here, which is not exactly the same as
       // DecimalQuantity::toScientificString(). The biggest difference is that uprv_decNumberToString does
       // not print scientific notation for magnitudes greater than -5 and smaller than some amount (+5?).
       if (fDecimalQuantity->isZero()) {
         fDecimalStr->append("0", -1, status);
-      } else if (std::abs(fDecimalQuantity->getMagnitude()) < 5) {
+      } else if (fDecimalQuantity->getMagnitude() != INT32_MIN && std::abs(fDecimalQuantity->getMagnitude()) < 5) {
         fDecimalStr->appendInvariantChars(fDecimalQuantity->toPlainString(), status);
       } else {
         fDecimalStr->appendInvariantChars(fDecimalQuantity->toScientificString(), status);
       }
     }
diff --git a/icu4c/source/i18n/number_decimalquantity.cpp b/icu4c/source/i18n/number_decimalquantity.cpp
index 47b930a564..d5dd7ae694 100644
--- a/icu4c/source/i18n/number_decimalquantity.cpp
+++ b/icu4c/source/i18n/number_decimalquantity.cpp
@@ -896,11 +896,14 @@ UnicodeString DecimalQuantity::toScientificString() const {
             result.append(u'0' + getDigitPos(p));
         }
     }
     result.append(u'E');
     int32_t _scale = upperPos + scale;
-    if (_scale < 0) {
+    if (_scale == INT32_MIN) {
+        result.append({u"-2147483648", -1});
+        return result;
+    } else if (_scale < 0) {
         _scale *= -1;
         result.append(u'-');
     } else {
         result.append(u'+');
     }
diff --git a/icu4c/source/test/intltest/numfmtst.cpp b/icu4c/source/test/intltest/numfmtst.cpp
index 3435593911..8d52dc122b 100644
--- a/icu4c/source/test/intltest/numfmtst.cpp
+++ b/icu4c/source/test/intltest/numfmtst.cpp
@@ -9224,10 +9224,18 @@ void NumberFormatTest::Test20037_ScientificIntegerOverflow() {
     nf->parse(u"1E-2147483647E-1", result, status);
     sp = result.getDecimalNumber(status);
     assertEquals(u"Should not overflow and should parse only the first exponent",
                  u"1E-2147483647",
                  {sp.data(), sp.length(), US_INV});
+
+    // Test edge case overflow of exponent
+    result = Formattable();
+    nf->parse(u".0003e-2147483644", result, status);
+    sp = result.getDecimalNumber(status);
+    assertEquals(u"Should not overflow",
+                 u"3E-2147483648",
+                 {sp.data(), sp.length(), US_INV});
 }
 
 void NumberFormatTest::Test13840_ParseLongStringCrash() {
     IcuTestErrorCode status(*this, "Test13840_ParseLongStringCrash");
 
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java
index f5fb4278e1..138c2e9acf 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_AbstractBCD.java
@@ -1065,11 +1065,14 @@ public void toScientificString(StringBuilder result) {
                 result.append((char) ('0' + getDigitPos(p)));
             }
         }
         result.append('E');
         int _scale = upperPos + scale;
-        if (_scale < 0) {
+        if (_scale == Integer.MIN_VALUE) {
+            result.append("-2147483648");
+            return;
+        } else if (_scale < 0) {
             _scale *= -1;
             result.append('-');
         } else {
             result.append('+');
         }
diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java
index 42cc2c48ec..faa46281a1 100644
--- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java
@@ -237,11 +237,19 @@ protected BigDecimal bcdToBigDecimal() {
             long tempLong = 0L;
             for (int shift = (precision - 1); shift >= 0; shift--) {
                 tempLong = tempLong * 10 + getDigitPos(shift);
             }
             BigDecimal result = BigDecimal.valueOf(tempLong);
-            result = result.scaleByPowerOfTen(scale);
+            try {
+                result = result.scaleByPowerOfTen(scale);
+            } catch (ArithmeticException e) {
+                if (e.getMessage().contains("Underflow")) {
+                    result = BigDecimal.ZERO;
+                } else {
+                    throw e;
+                }
+            }
             if (isNegative())
                 result = result.negate();
             return result;
         }
     }
diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java
index ce6b3666aa..fe0d77cca2 100644
--- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java
+++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java
@@ -6323,10 +6323,15 @@ public void Test20037_ScientificIntegerOverflow() throws ParseException {
 
         // For Java, we should get *pretty close* to 2^31.
         result = nf.parse("1E-547483647");
         assertEquals("Should *not* snap to zero",
                 "1E-547483647", result.toString());
+
+        // Test edge case overflow of exponent
+        result = nf.parse(".0003e-2147483644");
+        assertEquals("Should not overflow",
+                "0", result.toString());
     }
 
     @Test
     public void test13840_ParseLongStringCrash() throws ParseException {
         NumberFormat nf = NumberFormat.getInstance(ULocale.ENGLISH);

Vulnerable code:       } else if (std::abs(fDecimalQuantity->getMagnitude()) < 5) {
    if (_scale < 0) {
        if (_scale < 0) {
            result = result.scaleByPowerOfTen(scale);
Fixed code:       } else if (fDecimalQuantity->getMagnitude() != INT32_MIN && std::abs(fDecimalQuantity->getMagnitude()) < 5) {
    if (_scale == INT32_MIN) {
        result.append({u"-2147483648", -1});
        return result;
    } else if (_scale < 0) {

    // Test edge case overflow of exponent
    result = Formattable();
    nf->parse(u".0003e-2147483644", result, status);
    sp = result.getDecimalNumber(status);
    assertEquals(u"Should not overflow",
                 u"3E-2147483648",
                 {sp.data(), sp.length(), US_INV});
        if (_scale == Integer.MIN_VALUE) {
            result.append("-2147483648");
            return;
        } else if (_scale < 0) {
            try {
                result = result.scaleByPowerOfTen(scale);
            } catch (ArithmeticException e) {
                if (e.getMessage().contains("Underflow")) {
                    result = BigDecimal.ZERO;
                } else {
                    throw e;
                }
            }

        // Test edge case overflow of exponent
        result = nf.parse(".0003e-2147483644");
        assertEquals("Should not overflow",
                "0", result.toString());