diff --git a/icu4c/source/i18n/nfrs.cpp b/icu4c/source/i18n/nfrs.cpp index b8bd7a31cbf..5f0d6b3c1da 100644 --- a/icu4c/source/i18n/nfrs.cpp +++ b/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; } } diff --git a/icu4c/source/i18n/nfrule.cpp b/icu4c/source/i18n/nfrule.cpp index 247c0a31be4..e7908ecab91 100644 --- a/icu4c/source/i18n/nfrule.cpp +++ b/icu4c/source/i18n/nfrule.cpp @@ -19,6 +19,7 @@ #if U_HAVE_RBNF +#include #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 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 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(c - gZero); + int32_t single_digit = static_cast(c - gZero); + if ((val > 0 && val > (std::numeric_limits::max() - single_digit) / 10) || + (val < 0 && val < (std::numeric_limits::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; diff --git a/icu4c/source/test/intltest/itrbnf.cpp b/icu4c/source/test/intltest/itrbnf.cpp index fd2451856e5..c0d00ec8c34 100644 --- a/icu4c/source/test/intltest/itrbnf.cpp +++ b/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() { diff --git a/icu4c/source/test/intltest/itrbnf.h b/icu4c/source/test/intltest/itrbnf.h index fd6e4107817..3c8c4f1e024 100644 --- a/icu4c/source/test/intltest/itrbnf.h +++ b/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);