ICU-22636 Return U_BRK_RULE_SYNTAX when status number is too large

See #2793
unicode-org · Jan 19, 2024 · 7132c11 · 7132c11
1 parent 078b88a
commit 7132c11
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 2 deletions.
diff --git a/icu4c/source/common/rbbiscan.cpp b/icu4c/source/common/rbbiscan.cpp
@@ -530,7 +530,13 @@ UBool RBBIRuleScanner::doParseActions(int32_t action)
             n = fNodeStack[fNodeStackPtr];
             uint32_t v = u_charDigitValue(fC.fChar);
             U_ASSERT(v < 10);
-            n->fVal = n->fVal*10 + v;
+            int64_t updated = static_cast<int64_t>(n->fVal)*10 + v;
+            // Avoid overflow n->fVal
+            if (updated > INT32_MAX) {
+                error(U_BRK_RULE_SYNTAX);
+                break;
+            }
+            n->fVal = static_cast<int32_t>(updated);
             break;
         }
 

diff --git a/icu4c/source/test/intltest/rbbitst.cpp b/icu4c/source/test/intltest/rbbitst.cpp
@@ -149,6 +149,7 @@ void RBBITest::runIndexedTest( int32_t index, UBool exec, const char* &name, cha
     TESTCASE_AUTO(TestBug22584);
     TESTCASE_AUTO(TestBug22585);
     TESTCASE_AUTO(TestBug22602);
+    TESTCASE_AUTO(TestBug22636);
 
 #if U_ENABLE_TRACING
     TESTCASE_AUTO(TestTraceCreateCharacter);
@@ -5889,6 +5890,19 @@ void RBBITest::TestBug22602() {
     RuleBasedBreakIterator bi(rule, pe, ec);
 }
 
+void RBBITest::TestBug22636() {
+    UParseError pe {};
+    UErrorCode ec {U_ZERO_ERROR};
+    RuleBasedBreakIterator bi(u"A{77777777777777};", pe, ec);
+    assertEquals(WHERE, ec, U_BRK_RULE_SYNTAX);
+    ec = U_ZERO_ERROR;
+    RuleBasedBreakIterator bi2(u"A{2147483648};", pe, ec);
+    assertEquals(WHERE, ec, U_BRK_RULE_SYNTAX);
+    ec = U_ZERO_ERROR;
+    RuleBasedBreakIterator bi3(u"A{2147483647};", pe, ec);
+    assertEquals(WHERE, ec, U_ZERO_ERROR);
+}
+
 void RBBITest::TestBug22584() {
     // Creating a break iterator from a rule consisting of a very long
     // literal input string caused a stack overflow when deleting the

diff --git a/icu4c/source/test/intltest/rbbitst.h b/icu4c/source/test/intltest/rbbitst.h
@@ -103,6 +103,7 @@ class RBBITest: public IntlTest {
     void TestBug22584();
     void TestBug22585();
     void TestBug22602();
+    void TestBug22636();
 
 #if U_ENABLE_TRACING
     void TestTraceCreateCharacter();

diff --git a/icu4j/main/core/src/main/java/com/ibm/icu/text/RBBIRuleScanner.java b/icu4j/main/core/src/main/java/com/ibm/icu/text/RBBIRuleScanner.java
@@ -412,7 +412,11 @@ boolean doParseActions(int action) {
         {
             n = fNodeStack[fNodeStackPtr];
             int v = UCharacter.digit((char) fC.fChar, 10);
-            n.fVal = n.fVal * 10 + v;
+            long update = (long)(n.fVal) * 10 + v;
+            if (update > Integer.MAX_VALUE) {
+                error(RBBIRuleBuilder.U_BRK_RULE_SYNTAX);
+            }
+            n.fVal = (int)(update);
             break;
         }
 

diff --git a/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/rbbi/RBBITest.java b/icu4j/main/core/src/test/java/com/ibm/icu/dev/test/rbbi/RBBITest.java
@@ -1002,6 +1002,24 @@ public void TestBug22602() {
             fail("TestBug22602: Unexpected exception while new RuleBasedBreakIterator() with a long string followed by a ';': " + e);
         }
     }
+    @Test
+    public void TestBug22636() {
+        try {
+            RuleBasedBreakIterator bi = new RuleBasedBreakIterator("A{77777777777777};");
+            fail("TestBug22636: new RuleBasedBreakIterator() with a large status value inside {}:  should throw IllegalArgumentException");
+        }
+        catch (IllegalArgumentException e) {
+            // expected exception with a large status value inside {}.
+        }
+        try {
+            RuleBasedBreakIterator bi2 = new RuleBasedBreakIterator("A{2147483648};");
+            fail("TestBug22636: new RuleBasedBreakIterator() with a large status value inside {}:  should throw IllegalArgumentException");
+        }
+        catch (IllegalArgumentException e) {
+            // expected exception with a large status value inside {}.
+        }
+        RuleBasedBreakIterator bi3 = new RuleBasedBreakIterator("A{2147483647};");
+    }
     /* Test preceding(index) and following(index), with semi-random indexes.
      * The random indexes are produced in clusters that are relatively closely spaced,
      * to increase the occurrences of hits to the internal break cache.