ICU-22638 Use parseNumber to fix buffer-overflow

See #2795
This commit is contained in:
Frank Tang 2024-01-19 00:37:46 +00:00 committed by Frank Yung-Fong Tang
parent 7b690aa8c7
commit 3eb8923b97
6 changed files with 50 additions and 16 deletions

View file

@ -198,12 +198,11 @@ int32_t ICU_Utility::parseNumber(const UnicodeString& text,
if (d < 0) {
break;
}
n = radix*n + d;
// ASSUME that when a 32-bit integer overflows it becomes
// negative. E.g., 214748364 * 10 + 8 => negative value.
if (n < 0) {
int64_t update = radix*static_cast<int64_t>(n) + d;
if (update > INT32_MAX) {
return -1;
}
n = static_cast<int32_t>(update);
++p;
}
if (p == pos) {

View file

@ -678,20 +678,40 @@ PluralRuleParser::parse(const UnicodeString& ruleData, PluralRules *prules, UErr
U_ASSERT(curAndConstraint != nullptr);
if ( (curAndConstraint->op==AndConstraint::MOD)&&
(curAndConstraint->opNum == -1 ) ) {
curAndConstraint->opNum=getNumberValue(token);
int32_t num = getNumberValue(token);
if (num == -1) {
status = U_UNEXPECTED_TOKEN;
break;
}
curAndConstraint->opNum=num;
}
else {
if (curAndConstraint->rangeList == nullptr) {
// this is for an 'is' rule
curAndConstraint->value = getNumberValue(token);
int32_t num = getNumberValue(token);
if (num == -1) {
status = U_UNEXPECTED_TOKEN;
break;
}
curAndConstraint->value = num;
} else {
// this is for an 'in' or 'within' rule
if (curAndConstraint->rangeList->elementAti(rangeLowIdx) == -1) {
curAndConstraint->rangeList->setElementAt(getNumberValue(token), rangeLowIdx);
curAndConstraint->rangeList->setElementAt(getNumberValue(token), rangeHiIdx);
int32_t num = getNumberValue(token);
if (num == -1) {
status = U_UNEXPECTED_TOKEN;
break;
}
curAndConstraint->rangeList->setElementAt(num, rangeLowIdx);
curAndConstraint->rangeList->setElementAt(num, rangeHiIdx);
}
else {
curAndConstraint->rangeList->setElementAt(getNumberValue(token), rangeHiIdx);
int32_t num = getNumberValue(token);
if (num == -1) {
status = U_UNEXPECTED_TOKEN;
break;
}
curAndConstraint->rangeList->setElementAt(num, rangeHiIdx);
if (curAndConstraint->rangeList->elementAti(rangeLowIdx) >
curAndConstraint->rangeList->elementAti(rangeHiIdx)) {
// Range Lower bound > Range Upper bound.
@ -1263,13 +1283,8 @@ PluralRuleParser::~PluralRuleParser() {
int32_t
PluralRuleParser::getNumberValue(const UnicodeString& token) {
int32_t i;
char digits[128];
i = token.extract(0, token.length(), digits, UPRV_LENGTHOF(digits), US_INV);
digits[i]='\0';
return((int32_t)atoi(digits));
int32_t pos = 0;
return ICU_Utility::parseNumber(token, pos, 10);
}

View file

@ -994,6 +994,7 @@ group: number_output
# PluralRules internals:
unifiedcache
display_options
icu_utility_with_props
group: numberformatter
# ICU 60+ NumberFormatter API

View file

@ -70,6 +70,7 @@ void PluralRulesTest::runIndexedTest( int32_t index, UBool exec, const char* &na
TESTCASE_AUTO(testSelectTrailingZeros);
TESTCASE_AUTO(testLocaleExtension);
TESTCASE_AUTO(testDoubleEqualSign);
TESTCASE_AUTO(test22638LongNumberValue);
TESTCASE_AUTO_END;
}
@ -1065,6 +1066,16 @@ PluralRulesTest::testDoubleValue() {
}
}
void
PluralRulesTest::test22638LongNumberValue() {
IcuTestErrorCode errorCode(*this, "test22638LongNumberValue");
LocalPointer<PluralRules> pr(PluralRules::createRules(
u"g:c%4422322222232222222222232222222322222223222222232222222322222223"
u"2222222322222232222222322222223222232222222222222322222223222222",
errorCode));
errorCode.expectErrorAndReset(U_UNEXPECTED_TOKEN);
}
void
PluralRulesTest::testLongValue() {
IcuTestErrorCode errorCode(*this, "testLongValue");

View file

@ -51,6 +51,7 @@ private:
void testSelectTrailingZeros();
void testLocaleExtension();
void testDoubleEqualSign();
void test22638LongNumberValue();
void assertRuleValue(const UnicodeString& rule, double expected);
void assertRuleKeyValue(const UnicodeString& rule, const UnicodeString& key,

View file

@ -1757,4 +1757,11 @@ public class PluralRulesTest extends CoreTestFmwk {
form = xyz.select(range);
assertEquals("Fallback form", "other", form);
}
@Test
public void test22638LongNumberValue() {
PluralRules test = PluralRules.createRules(
"g:c%4422322222232222222222232222222322222223222222232222222322222223" +
"2222222322222232222222322222223222232222222222222322222223222222");
assertEquals("Long number value should get null", null, test);
}
}