Skip to content

Commit b43a417

Browse files
authored
GH-50057: [C++] Avoid signed overflow in Decimal FromString exponent (#50058)
### Rationale for this change `Decimal{32,64,128,256}::FromString` parse the exponent as `int32_t`, so a string like `0E-2147483648` reaches `DecimalFromString`/`SimpleDecimalFromString` with `dec.exponent == INT32_MIN` and negating it is UB: ``` decimal.cc: runtime error: negation of -2147483648 cannot be represented in type 'int32_t' (aka 'int') ``` A near-`INT32_MIN` exponent overflows the `-exponent + fractional_digits` addition the same way. Both helpers are reachable from the CSV/JSON readers parsing decimal columns. ### What changes are included in this PR? Compute the scale via `internal::SubtractWithOverflow` and reject the unrepresentable exponents with `Status::Invalid`, in both helpers. ### Are these changes tested? Yes, `0E-2147483648` and `1.0E-2147483647` added to `DecimalFromStringTest.InvalidInput` (runs for Decimal32/64/128/256). ### Are there any user-facing changes? No. * GitHub Issue: #50057 Lead-authored-by: metsw24-max <metsw24@gmail.com> Co-authored-by: Sayed Kaif <metsw24@gmail.com> Signed-off-by: Antoine Pitrou <antoine@python.org>
1 parent 12b3eda commit b43a417

2 files changed

Lines changed: 16 additions & 8 deletions

File tree

cpp/src/arrow/util/decimal.cc

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -881,9 +881,13 @@ Status DecimalFromString(const char* type_name, std::string_view s, Decimal* out
881881

882882
int32_t parsed_scale = 0;
883883
if (dec.has_exponent) {
884-
auto adjusted_exponent = dec.exponent;
885-
parsed_scale =
886-
-adjusted_exponent + static_cast<int32_t>(dec.fractional_digits.size());
884+
// parsed_scale = -exponent + fractional_digits, computed with overflow
885+
// detection: an exponent of INT32_MIN ("0E-2147483648") makes the negation,
886+
// and a near-INT32_MIN exponent the addition, signed-overflow UB otherwise.
887+
if (internal::SubtractWithOverflow(static_cast<int32_t>(dec.fractional_digits.size()),
888+
dec.exponent, &parsed_scale)) {
889+
return Status::Invalid("The string '", s, "' cannot be represented as ", type_name);
890+
}
887891
} else {
888892
parsed_scale = static_cast<int32_t>(dec.fractional_digits.size());
889893
}
@@ -945,9 +949,13 @@ Status SimpleDecimalFromString(const char* type_name, std::string_view s,
945949

946950
int32_t parsed_scale = 0;
947951
if (dec.has_exponent) {
948-
auto adjusted_exponent = dec.exponent;
949-
parsed_scale =
950-
-adjusted_exponent + static_cast<int32_t>(dec.fractional_digits.size());
952+
// parsed_scale = -exponent + fractional_digits, computed with overflow
953+
// detection: an exponent of INT32_MIN ("0E-2147483648") makes the negation,
954+
// and a near-INT32_MIN exponent the addition, signed-overflow UB otherwise.
955+
if (internal::SubtractWithOverflow(static_cast<int32_t>(dec.fractional_digits.size()),
956+
dec.exponent, &parsed_scale)) {
957+
return Status::Invalid("The string '", s, "' cannot be represented as ", type_name);
958+
}
951959
} else {
952960
parsed_scale = static_cast<int32_t>(dec.fractional_digits.size());
953961
}

cpp/src/arrow/util/decimal_test.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,8 @@ class DecimalFromStringTest : public ::testing::Test {
135135
void TestInvalidInput() {
136136
for (const std::string invalid_value :
137137
{"-", "0.0.0", "0-13-32", "a", "-23092.235-", "-+23092.235", "+-23092.235",
138-
"00a", "1e1a", "0.00123D/3", "1.23eA8", "1.23E+3A", "-1.23E--5",
139-
"1.2345E+++07"}) {
138+
"00a", "1e1a", "0.00123D/3", "1.23eA8", "1.23E+3A", "-1.23E--5", "1.2345E+++07",
139+
"0E-2147483648", "1.0E-2147483647"}) {
140140
ARROW_SCOPED_TRACE("invalid_value = '", invalid_value, "'");
141141
ASSERT_RAISES(Invalid, DecimalType::FromString(invalid_value));
142142
}

0 commit comments

Comments
 (0)