ICU-22897 Fix memory leak and int overflow

1. Rewrite to use LocalPointer to prevent memory leak
2. Rewrite the if/else to switch to make the logic clear
3. Delete the rule if not remember inside the rule set to fix memory
leak.
4. Check base value calculation to avoid int64_t overflow.
5. Add memory leak test

Author: FrankYFTang

icu4c/source/i18n/nfrs.cpp

@@ -267,27 +267,35 @@ NFRuleSet::parseRules(UnicodeString& description, UErrorCode& status)
  * @param rule The rule to set.
  */
 void NFRuleSet::setNonNumericalRule(NFRule *rule) {
-    int64_t baseValue = rule->getBaseValue();
-    if (baseValue == NFRule::kNegativeNumberRule) {
-        delete nonNumericalRules[NEGATIVE_RULE_INDEX];
-        nonNumericalRules[NEGATIVE_RULE_INDEX] = rule;
-    }
-    else if (baseValue == NFRule::kImproperFractionRule) {
-        setBestFractionRule(IMPROPER_FRACTION_RULE_INDEX, rule, true);
-    }
-    else if (baseValue == NFRule::kProperFractionRule) {
-        setBestFractionRule(PROPER_FRACTION_RULE_INDEX, rule, true);
-    }
-    else if (baseValue == NFRule::kDefaultRule) {
-        setBestFractionRule(DEFAULT_RULE_INDEX, rule, true);
-    }
-    else if (baseValue == NFRule::kInfinityRule) {
-        delete nonNumericalRules[INFINITY_RULE_INDEX];
-        nonNumericalRules[INFINITY_RULE_INDEX] = rule;
-    }
-    else if (baseValue == NFRule::kNaNRule) {
-        delete nonNumericalRules[NAN_RULE_INDEX];
-        nonNumericalRules[NAN_RULE_INDEX] = rule;
+    switch (rule->getBaseValue()) {
+        case NFRule::kNegativeNumberRule:
+            delete nonNumericalRules[NEGATIVE_RULE_INDEX];
+            nonNumericalRules[NEGATIVE_RULE_INDEX] = rule;
+            return;
+        case NFRule::kImproperFractionRule:
+            setBestFractionRule(IMPROPER_FRACTION_RULE_INDEX, rule, true);
+            return;
+        case NFRule::kProperFractionRule:
+            setBestFractionRule(PROPER_FRACTION_RULE_INDEX, rule, true);
+            return;
+        case NFRule::kDefaultRule:
+            setBestFractionRule(DEFAULT_RULE_INDEX, rule, true);
+            return;
+        case NFRule::kInfinityRule:
+            delete nonNumericalRules[INFINITY_RULE_INDEX];
+            nonNumericalRules[INFINITY_RULE_INDEX] = rule;
+            return;
+        case NFRule::kNaNRule:
+            delete nonNumericalRules[NAN_RULE_INDEX];
+            nonNumericalRules[NAN_RULE_INDEX] = rule;
+            return;
+        case NFRule::kNoBase:
+        case NFRule::kOtherRule:
+        default:
+            // If we do not remember the rule inside the object.
+            // delete it here to prevent memory leak.
+            delete rule;
+            return;
     }
 }
 

icu4c/source/i18n/nfrule.cpp

@@ -19,6 +19,7 @@
 
 #if U_HAVE_RBNF
 
+#include <limits>
 #include "unicode/localpointer.h"
 #include "unicode/rbnf.h"
 #include "unicode/tblcoll.h"
@@ -116,9 +117,9 @@ NFRule::makeRules(UnicodeString& description,
     // new it up and initialize its basevalue and divisor
     // (this also strips the rule descriptor, if any, off the
     // description string)
-    NFRule* rule1 = new NFRule(rbnf, description, status);
+    LocalPointer<NFRule> rule1(new NFRule(rbnf, description, status));
     /* test for nullptr */
-    if (rule1 == nullptr) {
+    if (rule1.isNull()) {
         status = U_MEMORY_ALLOCATION_ERROR;
         return;
     }
@@ -144,7 +145,7 @@ NFRule::makeRules(UnicodeString& description,
     else {
         // if the description does contain a matched pair of brackets,
         // then it's really shorthand for two rules (with one exception)
-        NFRule* rule2 = nullptr;
+        LocalPointer<NFRule> rule2;
         UnicodeString sbuf;
 
         // we'll actually only split the rule into two rules if its
@@ -160,9 +161,9 @@ NFRule::makeRules(UnicodeString& description,
             // set, they both have the same base value; otherwise,
             // increment the original rule's base value ("rule1" actually
             // goes SECOND in the rule set's rule list)
-            rule2 = new NFRule(rbnf, UnicodeString(), status);
+            rule2.adoptInstead(new NFRule(rbnf, UnicodeString(), status));
             /* test for nullptr */
-            if (rule2 == nullptr) {
+            if (rule2.isNull()) {
                 status = U_MEMORY_ALLOCATION_ERROR;
                 return;
             }
@@ -217,20 +218,20 @@ NFRule::makeRules(UnicodeString& description,
         // BEFORE rule1 in the list: in all cases, rule2 OMITS the
         // material in the brackets and rule1 INCLUDES the material
         // in the brackets)
-        if (rule2 != nullptr) {
+        if (!rule2.isNull()) {
             if (rule2->baseValue >= kNoBase) {
-                rules.add(rule2);
+                rules.add(rule2.orphan());
             }
             else {
-                owner->setNonNumericalRule(rule2);
+                owner->setNonNumericalRule(rule2.orphan());
             }
         }
     }
     if (rule1->baseValue >= kNoBase) {
-        rules.add(rule1);
+        rules.add(rule1.orphan());
     }
     else {
-        owner->setNonNumericalRule(rule1);
+        owner->setNonNumericalRule(rule1.orphan());
     }
 }
 
@@ -289,7 +290,14 @@ NFRule::parseRuleDescriptor(UnicodeString& description, UErrorCode& status)
             while (p < descriptorLength) {
                 c = descriptor.charAt(p);
                 if (c >= gZero && c <= gNine) {
-                    val = val * ll_10 + static_cast<int32_t>(c - gZero);
+                    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)) {
+                        // out of int64_t range
+                        status = U_PARSE_ERROR;
+                        return;
+                    }
+                    val = val * ll_10 + single_digit;
                 }
                 else if (c == gSlash || c == gGreaterThan) {
                     break;

icu4c/source/test/intltest/itrbnf.cpp

@@ -80,6 +80,7 @@ void IntlTestRBNF::runIndexedTest(int32_t index, UBool exec, const char* &name,
         TESTCASE(28, TestNorwegianSpellout);
         TESTCASE(29, TestNumberingSystem);
         TESTCASE(30, TestDFRounding);
+        TESTCASE(31, TestMemoryLeak22899);
 #else
         TESTCASE(0, TestRBNFDisabled);
 #endif
@@ -1340,6 +1341,14 @@ void IntlTestRBNF::TestDFRounding()
     }
 }
 
+void IntlTestRBNF::TestMemoryLeak22899()
+{
+    UErrorCode status = U_ZERO_ERROR;
+    UParseError perror;
+    icu::UnicodeString str(u"0,31,01,30,01,0,01,01,30,01,30,31,01,30,01,30,30,00,01,0:");
+    icu::RuleBasedNumberFormat rbfmt(str, icu::Locale::getEnglish(), perror, status);
+}
+
 void 
 IntlTestRBNF::TestSpanishSpellout() 
 {

icu4c/source/test/intltest/itrbnf.h

@@ -161,6 +161,7 @@ class IntlTestRBNF : public IntlTest {
     void TestParseFailure();
     void TestMinMaxIntegerDigitsIgnored();
     void TestNumberingSystem();
+    void TestMemoryLeak22899();
 
 protected:
   virtual void doTest(RuleBasedNumberFormat* formatter, const char* const testData[][2], UBool testParsing);