ICU-23002 Fix int64_t overflow in NFRule::parseRuleDescriptor

See #3324
unicode-org · Jan 3, 2025 · 93ce388 · 93ce388
1 parent ed69e53
commit 93ce388
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 8 deletions.
diff --git a/icu4c/source/i18n/nfrule.cpp b/icu4c/source/i18n/nfrule.cpp
@@ -19,7 +19,6 @@
 
 #if U_HAVE_RBNF
 
-#include <limits>
 #include "unicode/localpointer.h"
 #include "unicode/rbnf.h"
 #include "unicode/tblcoll.h"
@@ -286,18 +285,17 @@ NFRule::parseRuleDescriptor(UnicodeString& description, UErrorCode& status)
             // into "tempValue", skip periods, commas, and spaces,
             // stop on a slash or > sign (or at the end of the string),
             // and throw an exception on any other character
-            int64_t ll_10 = 10;
             while (p < descriptorLength) {
                 c = descriptor.charAt(p);
                 if (c >= gZero && c <= gNine) {
-                    int32_t single_digit = static_cast<int32_t>(c - gZero);
-                    if ((val > 0 && val > (std::numeric_limits<int64_t>::max() - single_digit) / 10) ||
-                        (val < 0 && val < (std::numeric_limits<int64_t>::min() - single_digit) / 10)) {
+                    int64_t digit = static_cast<int64_t>(c - gZero);
+                    if ((val > 0 && val > (INT64_MAX - digit) / 10) ||
+                        (val < 0 && val < (INT64_MIN - digit) / 10)) {
                         // out of int64_t range
                         status = U_PARSE_ERROR;
                         return;
                     }
-                    val = val * ll_10 + single_digit;
+                    val = val * 10 + digit;
                 }
                 else if (c == gSlash || c == gGreaterThan) {
                     break;
@@ -322,11 +320,17 @@ NFRule::parseRuleDescriptor(UnicodeString& description, UErrorCode& status)
             if (c == gSlash) {
                 val = 0;
                 ++p;
-                ll_10 = 10;
                 while (p < descriptorLength) {
                     c = descriptor.charAt(p);
                     if (c >= gZero && c <= gNine) {
-                        val = val * ll_10 + static_cast<int32_t>(c - gZero);
+                        int64_t digit = static_cast<int64_t>(c - gZero);
+                        if ((val > 0 && val > (INT64_MAX - digit) / 10) ||
+                            (val < 0 && val < (INT64_MIN - digit) / 10)) {
+                            // out of int64_t range
+                            status = U_PARSE_ERROR;
+                            return;
+                        }
+                        val = val * 10 + digit;
                     }
                     else if (c == gGreaterThan) {
                         break;

diff --git a/icu4c/source/test/intltest/itrbnf.cpp b/icu4c/source/test/intltest/itrbnf.cpp
@@ -82,6 +82,7 @@ void IntlTestRBNF::runIndexedTest(int32_t index, UBool exec, const char* &name,
         TESTCASE(30, TestDFRounding);
         TESTCASE(31, TestMemoryLeak22899);
         TESTCASE(32, TestInfiniteRecursion);
+        TESTCASE(33, TestParseRuleDescriptorOverflow23002);
 #else
         TESTCASE(0, TestRBNFDisabled);
 #endif
@@ -2615,6 +2616,18 @@ IntlTestRBNF::TestNumberingSystem() {
     }
 }
 
+void
+IntlTestRBNF::TestParseRuleDescriptorOverflow23002() {
+    UParseError perror;
+    UErrorCode status = U_ZERO_ERROR;
+    // Test int64 overflow inside parseRuleDescriptor
+    UnicodeString testStr(u"0110110/300113001103000113001103000110i/3013033:");
+    icu::RuleBasedNumberFormat rbfmt(
+        testStr,
+        Locale("as"), perror, status);
+    assertEquals("number too large", U_PARSE_ERROR, status);
+}
+
 void
 IntlTestRBNF::TestInfiniteRecursion() {
     UnicodeString badRules[] = {

diff --git a/icu4c/source/test/intltest/itrbnf.h b/icu4c/source/test/intltest/itrbnf.h
@@ -163,6 +163,7 @@ class IntlTestRBNF : public IntlTest {
     void TestNumberingSystem();
     void TestMemoryLeak22899();
     void TestInfiniteRecursion();
+    void TestParseRuleDescriptorOverflow23002();
 
 protected:
   virtual void doTest(RuleBasedNumberFormat* formatter, const char* const testData[][2], UBool testParsing);

diff --git a/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/format/RBNFParseTest.java b/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/format/RBNFParseTest.java
@@ -165,6 +165,18 @@ public void TestLenientParse() throws Exception {
         parseList(rbnf_en, rbnf_fr, lists);
     }
 
+    @Test
+    public void TestParseRuleDescriptorOverflow23002() {
+        try {
+            RuleBasedNumberFormat rbnf =
+                new RuleBasedNumberFormat(
+                    "0110110/300113001103000113001103000110i/3013033:",
+                    new Locale("as"));
+        } catch (IllegalArgumentException e) {
+            return;
+        }
+        errln("expected exception but didn't get one!");
+    }
     @Test
     public void TestBadParse() {
         RuleBasedNumberFormat rbnf = new RuleBasedNumberFormat(Locale.JAPAN, RuleBasedNumberFormat.SPELLOUT);