Skip to content

Commit d091957

Browse files
authored
GH-50075: [C++][Gandiva] fix buffer overrun in to_hex int32/int64 (#50076)
### Rationale for this change `to_hex_int64` and `to_hex_int32` in `cpp/src/gandiva/precompiled/string_ops.cc` allocate the arena buffer with the bare hex-digit count (16 and 8 bytes) but pass that size + 1 as the `snprintf` bound. For any full-width value (`to_hex(-1::bigint)` -> `FFFFFFFFFFFFFFFF`, `INT64_MIN`, `INT32_MIN`, ...) `snprintf` writes all digits plus the trailing NUL, one byte past the end of the allocation. `gdv_fn_context_arena_malloc` returns exactly the requested bytes, so the NUL corrupts the adjacent arena allocation. Reachable from the `to_hex(bigint)` / `to_hex(int)` Gandiva functions over untrusted column data. ### What changes are included in this PR? Allocate one extra byte in both functions so the allocation matches the `snprintf` bound, the same fix as GH-49752 applied to the identical pattern in `hash_utils.cc`. ### Are these changes tested? Yes. The existing `TestToHexInt64` / `TestToHexInt32` cases already exercise the full-width path (`INT64_MIN`, `INT32_MIN`, -1) and run clean under ASAN with this change. Output behaviour is unchanged. ### Are there any user-facing changes? No. **This PR contains a "Critical Fix".** It fixes a one-byte out-of-bounds heap write reachable from the `to_hex` Gandiva functions over untrusted input. * GitHub Issue: #50075 Authored-by: metsw24-max <metsw24@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
1 parent cffe22d commit d091957

1 file changed

Lines changed: 5 additions & 3 deletions

File tree

cpp/src/gandiva/precompiled/string_ops.cc

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2895,8 +2895,9 @@ const char* to_hex_binary(int64_t context, const char* text, int32_t text_len,
28952895
FORCE_INLINE
28962896
const char* to_hex_int64(int64_t context, int64_t data, int32_t* out_len) {
28972897
const int64_t hex_long_max_size = 2 * sizeof(int64_t);
2898-
auto ret =
2899-
reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context, hex_long_max_size));
2898+
// Allocate one extra byte for the null terminator written by snprintf.
2899+
auto ret = reinterpret_cast<char*>(
2900+
gdv_fn_context_arena_malloc(context, hex_long_max_size + 1));
29002901

29012902
if (ret == nullptr) {
29022903
gdv_fn_context_set_error_msg(context, "Could not allocate memory for output string");
@@ -2912,7 +2913,8 @@ const char* to_hex_int64(int64_t context, int64_t data, int32_t* out_len) {
29122913
FORCE_INLINE
29132914
const char* to_hex_int32(int64_t context, int32_t data, int32_t* out_len) {
29142915
const int32_t max_size = 2 * sizeof(int32_t);
2915-
auto ret = reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context, max_size));
2916+
// Allocate one extra byte for the null terminator written by snprintf.
2917+
auto ret = reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context, max_size + 1));
29162918

29172919
if (ret == nullptr) {
29182920
gdv_fn_context_set_error_msg(context, "Could not allocate memory for output string");

0 commit comments

Comments
 (0)