You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ko...@apache.org on 2026/10/16 01:12:52 UTC
(arrow) branch main updated: GH-50544: [C++][Gandiva] Fix out-of-bounds read in round with most negative precision (#50545)
This is an automated email from the ASF dual-hosted git repository.
kou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new d26582eee4e GH-50544: [C++][Gandiva] Fix out-of-bounds read in round with most negative precision (#50545)
d26582eee4e is described below
commit d26582eee4e07c1ad3e840757ed35e835596bb5d
Author: Abdul Rawoof Khan <ab...@bugqore.com>
AuthorDate: Fri Oct 16 06:42:45 2026 +0530
GH-50544: [C++][Gandiva] Fix out-of-bounds read in round with most negative precision (#50545)
### Rationale for this change
`round_int32_int32` and `round_int64_int32` negate the caller-supplied precision into `abs_precision` before range-checking it, so the check `abs_precision > 9` (and `> 18` for the int64 overload) never fires for `precision == INT32_MIN`, where the negation leaves the value negative. That value reaches `get_power_of_10`, which indexes a 19-element static table behind `DCHECK_GE`/`DCHECK_LE` only. The DCHECKs compile out in release builds, so `round(int_col, -2147483648)` reads far out [...]
`truncate_int64_int32` in the same file already compares the signed scale directly (`out_scale < -38`) and is unaffected; the two `round` overloads are the only sites that negate first.
### What changes are included in this PR?
Check the bound against the signed `precision` before negating, at both sites. Every precision that was in range before still takes the same path, and the negation now only runs on values known to be within 9 (or 18) of zero.
### Are these changes tested?
Yes. Added two cases to `TestExtendedMathOps.TestRound` covering `std::numeric_limits<int32_t>::min()` for both overloads, matching the existing `truncate_int64_int32` case in the same file. On the unpatched tree the debug build aborts with `extended_math_ops.cc:372: Check failed: (exp) >= (0)`; with the fix the full `gandiva-precompiled-test` suite passes (134 tests).
### Are there any user-facing changes?
No.
**This PR contains a "Critical Fix".** It fixes an out-of-bounds read reachable from a user-supplied `round` precision argument.
* GitHub Issue: #50544
Authored-by: abdul rawoof <ab...@bugqore.com>
Signed-off-by: Sutou Kouhei <ko...@clear-code.com>
---
cpp/src/gandiva/precompiled/extended_math_ops.cc | 14 ++++++++------
cpp/src/gandiva/precompiled/extended_math_ops_test.cc | 12 ++++++++++++
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/cpp/src/gandiva/precompiled/extended_math_ops.cc b/cpp/src/gandiva/precompiled/extended_math_ops.cc
index 348d595faf4..206295f06dd 100644
--- a/cpp/src/gandiva/precompiled/extended_math_ops.cc
+++ b/cpp/src/gandiva/precompiled/extended_math_ops.cc
@@ -324,12 +324,13 @@ gdv_int32 round_int32_int32(gdv_int32 number, gdv_int32 precision) {
if (precision >= 0) {
return number;
}
- gdv_int32 abs_precision = -precision;
// This is to ensure that there is no overflow while calculating 10^precision, 9 is
- // the smallest N for which 10^N does not fit into 32 bits, so we can safely return 0
- if (abs_precision > 9) {
+ // the smallest N for which 10^N does not fit into 32 bits, so we can safely return 0.
+ // The bound is checked before negating because -INT32_MIN is still negative.
+ if (precision < -9) {
return 0;
}
+ gdv_int32 abs_precision = -precision;
gdv_int32 num_sign = (number > 0) ? 1 : -1;
gdv_int32 abs_number = number * num_sign;
gdv_int32 power_of_10 = static_cast<gdv_int32>(get_power_of_10(abs_precision));
@@ -349,12 +350,13 @@ gdv_int64 round_int64_int32(gdv_int64 number, gdv_int32 precision) {
if (precision >= 0) {
return number;
}
- gdv_int32 abs_precision = -precision;
// This is to ensure that there is no overflow while calculating 10^precision, 19 is
- // the smallest N for which 10^N does not fit into 64 bits, so we can safely return 0
- if (abs_precision > 18) {
+ // the smallest N for which 10^N does not fit into 64 bits, so we can safely return 0.
+ // The bound is checked before negating because -INT32_MIN is still negative.
+ if (precision < -18) {
return 0;
}
+ gdv_int32 abs_precision = -precision;
gdv_int32 num_sign = (number > 0) ? 1 : -1;
gdv_int64 abs_number = number * num_sign;
gdv_int64 power_of_10 = get_power_of_10(abs_precision);
diff --git a/cpp/src/gandiva/precompiled/extended_math_ops_test.cc b/cpp/src/gandiva/precompiled/extended_math_ops_test.cc
index e7a0f74c1b4..f9c92dd0911 100644
--- a/cpp/src/gandiva/precompiled/extended_math_ops_test.cc
+++ b/cpp/src/gandiva/precompiled/extended_math_ops_test.cc
@@ -203,6 +203,18 @@ TEST(TestExtendedMathOps, TestRound) {
EXPECT_EQ(round_int64_int32(-23453462343, -4), -23453460000);
EXPECT_EQ(round_int64_int32(-23453462343, -5), -23453500000);
EXPECT_EQ(round_int64_int32(345353425343, -12), 0);
+
+ // The last in-range precision still rounds; one past it returns 0. These guard
+ // against an off-by-one in the range check at both overloads' boundary.
+ EXPECT_EQ(round_int32_int32(1234567890, -9), 1000000000);
+ EXPECT_EQ(round_int32_int32(1234567890, -10), 0);
+ EXPECT_EQ(round_int64_int32(1234567890123456789, -18), 1000000000000000000);
+ EXPECT_EQ(round_int64_int32(1234567890123456789, -19), 0);
+
+ // The most negative precision must be rejected by the range check rather than
+ // negated into an out-of-range table index.
+ EXPECT_EQ(round_int32_int32(1234, std::numeric_limits<int32_t>::min()), 0);
+ EXPECT_EQ(round_int64_int32(345353425343, std::numeric_limits<int32_t>::min()), 0);
}
TEST(TestExtendedMathOps, TestTruncate) {