From 73ca6dd1939f59acd03d9f82fed153dc2924a4e9 Mon Sep 17 00:00:00 2001 From: abdul rawoof Date: Mon, 24 Aug 2026 14:37:55 +0530 Subject: [PATCH] GH-50964: [C++] fix out-of-bounds read in DecimalRescale for out-of-range scale --- cpp/src/arrow/util/basic_decimal.cc | 10 ++++++++++ cpp/src/arrow/util/decimal_test.cc | 21 +++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/cpp/src/arrow/util/basic_decimal.cc b/cpp/src/arrow/util/basic_decimal.cc index eddb1aae7b2d..a764b05b89d2 100644 --- a/cpp/src/arrow/util/basic_decimal.cc +++ b/cpp/src/arrow/util/basic_decimal.cc @@ -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); diff --git a/cpp/src/arrow/util/decimal_test.cc b/cpp/src/arrow/util/decimal_test.cc index 7022c8117802..585b4eb8d1a5 100644 --- a/cpp/src/arrow/util/decimal_test.cc +++ b/cpp/src/arrow/util/decimal_test.cc @@ -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( @@ -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) {