Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions cpp/src/arrow/util/basic_decimal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1061,6 +1061,16 @@ DecimalStatus DecimalRescale(const DecimalClass& value, int32_t original_scale,
}

const int32_t delta_scale = new_scale - original_scale;

// GetScaleMultiplier only covers scales in [0, kMaxScale]; a larger delta would
// index the scale-multiplier table out of bounds. A rescale that big can never be
// represented without overflow or truncation, so reject it here instead of reading
// past the table. Testing delta_scale before std::abs also avoids abs(INT32_MIN).
if (ARROW_PREDICT_FALSE(delta_scale > DecimalClass::kMaxScale ||
delta_scale < -DecimalClass::kMaxScale)) {
return DecimalStatus::kRescaleDataLoss;
}

const int32_t abs_delta_scale = std::abs(delta_scale);

DecimalClass multiplier = DecimalClass::GetScaleMultiplier(abs_delta_scale);
Expand Down
21 changes: 21 additions & 0 deletions cpp/src/arrow/util/decimal_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,20 @@ TEST(Decimal128Test, FromStringLimits) {
38, 38);
}

TEST(Decimal128Test, RescaleScaleParsedFromStringOutOfRange) {
// A negative exponent with few significant digits parses to a small precision but
// a scale far beyond kMaxScale. Rescaling that value to the target scale (as the
// CSV decimal converter does) used to index the scale-multiplier table out of
// bounds; it now fails cleanly.
Decimal128 value;
int32_t precision = 0;
int32_t scale = 0;
ASSERT_OK(Decimal128::FromString("1E-100", &value, &precision, &scale));
ASSERT_EQ(precision, 1);
ASSERT_EQ(scale, 100);
ASSERT_RAISES(Invalid, value.Rescale(scale, 0));
}

TEST(Decimal256Test, FromStringLimits) {
// Positive / zero exponent
AssertDecimalFromString(
Expand Down Expand Up @@ -1599,6 +1613,13 @@ TYPED_TEST(TestBasicDecimalFunctionality, Rescale) {
ASSERT_OK_AND_EQ(TypeParam(-1), negative_value.Rescale(new_scale, original_scale));
}
}

// A scale change larger than kMaxScale used to index the scale-multiplier table
// out of bounds (an out-of-bounds read caught by ASAN). It now reports data loss
// for both the multiply and divide directions. A delta of exactly kMaxScale is the
// largest valid multiplier and is exercised by the loops above.
ASSERT_RAISES(Invalid, TypeParam(1).Rescale(0, TypeParam::kMaxScale + 1));
ASSERT_RAISES(Invalid, TypeParam(1).Rescale(TypeParam::kMaxScale + 1, 0));
}

TYPED_TEST(TestBasicDecimalFunctionality, Mod) {
Expand Down
Loading