ICU-20037 Fixing ScientificMatcher integer overflow. (#138)

Also restricts parsing to read only one exponent per string.

diff --git a/icu4c/source/i18n/numparse_scientific.cpp b/icu4c/source/i18n/numparse_scientific.cpp
index 611695e57d4743ccd7e117eda1bcbd662a151977..de38957440817cdcffdd5a952b8a38b3f1b5089c 100644 (file)
--- a/icu4c/source/i18n/numparse_scientific.cpp
+++ b/icu4c/source/i18n/numparse_scientific.cpp
@@ -56,6 +56,11 @@ bool ScientificMatcher::match(StringSegment& segment, ParsedNumber& result, UErr
         return false;
     }
 
+    // Only accept one exponent per string.
+    if (0 != (result.flags & FLAG_HAS_EXPONENT)) {
+        return false;
+    }
+
     // First match the scientific separator, and then match another number after it.
     // NOTE: This is guarded by the smoke test; no need to check fExponentSeparatorString length again.
     int overlap1 = segment.getCommonPrefixLength(fExponentSeparatorString);

diff --git a/icu4c/source/test/intltest/numfmtst.cpp b/icu4c/source/test/intltest/numfmtst.cpp
index db8d22728b18903342ae262a636bd1223a4ac482..77d4283441b21055fcb2ffc5c76cbc7e451775fe 100644 (file)
--- a/icu4c/source/test/intltest/numfmtst.cpp
+++ b/icu4c/source/test/intltest/numfmtst.cpp
@@ -215,6 +215,7 @@ void NumberFormatTest::runIndexedTest( int32_t index, UBool exec, const char* &n
   TESTCASE_AUTO(Test13763_FieldPositionIteratorOffset);
   TESTCASE_AUTO(Test13777_ParseLongNameNonCurrencyMode);
   TESTCASE_AUTO(Test13804_EmptyStringsWhenParsing);
+  TESTCASE_AUTO(Test20037_ScientificIntegerOverflow);
   TESTCASE_AUTO(Test13840_ParseLongStringCrash);
   TESTCASE_AUTO(Test13850_EmptyStringCurrency);
   TESTCASE_AUTO_END;
@@ -9185,6 +9186,28 @@ void NumberFormatTest::Test13804_EmptyStringsWhenParsing() {
     }
 }
 
+void NumberFormatTest::Test20037_ScientificIntegerOverflow() {
+    IcuTestErrorCode status(*this, "Test20037_ScientificIntegerOverflow");
+
+    LocalPointer<NumberFormat> nf(NumberFormat::createInstance(status));
+    Formattable result;
+
+    // Test overflow of exponent
+    nf->parse(u"1E-2147483648", result, status);
+    StringPiece sp = result.getDecimalNumber(status);
+    assertEquals(u"Should snap to zero",
+        u"0",
+        {sp.data(), sp.length(), US_INV});
+
+    // Test edge case overflow of exponent
+    result = Formattable();
+    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});
+}
+
 void NumberFormatTest::Test13840_ParseLongStringCrash() {
     IcuTestErrorCode status(*this, "Test13840_ParseLongStringCrash");
 

diff --git a/icu4c/source/test/intltest/numfmtst.h b/icu4c/source/test/intltest/numfmtst.h
index 9f7a5c083534b5b3a090b4d0fa409903300fce0f..65e0ffe8f3374ff65fc634b0a60d655dee6acce2 100644 (file)
--- a/icu4c/source/test/intltest/numfmtst.h
+++ b/icu4c/source/test/intltest/numfmtst.h
@@ -279,6 +279,7 @@ class NumberFormatTest: public CalendarTimeZoneTest {
     void Test13763_FieldPositionIteratorOffset();
     void Test13777_ParseLongNameNonCurrencyMode();
     void Test13804_EmptyStringsWhenParsing();
+    void Test20037_ScientificIntegerOverflow();
     void Test13840_ParseLongStringCrash();
     void Test13850_EmptyStringCurrency();
 

diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/ScientificMatcher.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/ScientificMatcher.java
index 1904ea414a21921910d042aed0f8b9084cfe69a7..866d2dad8ebf1ad1d9390749286a9bb1b99ca306 100644 (file)
--- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/ScientificMatcher.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/ScientificMatcher.java
@@ -54,6 +54,11 @@ public class ScientificMatcher implements NumberParseMatcher {
             return false;
         }
 
+        // Only accept one exponent per string.
+        if (0 != (result.flags & ParsedNumber.FLAG_HAS_EXPONENT)) {
+            return false;
+        }
+
         // First match the scientific separator, and then match another number after it.
         // NOTE: This is guarded by the smoke test; no need to check exponentSeparatorString length again.
         int overlap1 = segment.getCommonPrefixLength(exponentSeparatorString);

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 5e67d35952d18ccc85aed78cb30ae243c22c719a..d276f1fc0b255157d4a76c021578b2cc894b624f 100644 (file)
--- 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
@@ -6294,6 +6294,28 @@ public class NumberFormatTest extends TestFmwk {
         }
     }
 
+    @Test
+    public void Test20037_ScientificIntegerOverflow() throws ParseException {
+        NumberFormat nf = NumberFormat.getInstance(ULocale.US);
+
+        // Test overflow of exponent
+        Number result = nf.parse("1E-2147483648");
+        assertEquals("Should snap to zero",
+                "0", result.toString());
+
+        // Test edge case overflow of exponent
+        // Note: the behavior is different from C++; this is probably due to the
+        // intermediate BigDecimal form, which has its own restrictions
+        result = nf.parse("1E-2147483647E-1");
+        assertEquals("Should not overflow and should parse only the first exponent",
+                "0.0", result.toString());
+
+        // 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
     public void test13840_ParseLongStringCrash() throws ParseException {
         NumberFormat nf = NumberFormat.getInstance(ULocale.ENGLISH);