Repository navigation
Conversation
In mask_utf8_utf8_utf8_utf8, the output buffer size is computed as std::max(upper_length, std::max(lower_length, num_length)) * data_len without an overflow guard. The multiplication is done in int32_t, so a large replacement string combined with a large input wraps around, producing a small or negative max_length. The arena allocation is then far smaller than needed, causing a heap buffer overflow in the subsequent memcpy loop. Fix: use arrow::internal::MultiplyWithOverflow to detect the overflow and return an error, consistent with sibling functions (repeat_utf8_int32, to_hex_binary, etc.). Fixes apache#50472
|
Thanks for opening a pull request! This pull request has been automatically converted to a draft because its title doesn't match Arrow's required format. If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename the pull request title in the following format? or After updating the title, you can mark the pull request as ready for review. See also: |
github-actions
Bot
added
Component: C++
Component: Gandiva
awaiting review
gdv_fn_upper_utf8, gdv_fn_lower_utf8, and gdv_fn_initcap_utf8 call arrow::util::UTF8Decode without checking that the multibyte sequence fits within data_len. When the input ends in a truncated multibyte sequence, UTF8Decode reads past the buffer. Add bounds check before UTF8Decode, consistent with the invalid UTF-8 error handling.
VrtxOmega
changed the title
Avoid addition overflow in the slice bound checks. Cover output-size multiplication and partial multibyte characters, and correct a valid InitCap fixture that supplied 19 for a 20-byte input. Assisted-by: Codex:GPT-6
VrtxOmega
changed the title
| // Ensure the multibyte sequence fits within the buffer to avoid | ||
| // reading past data_len (truncated trailing multibyte sequence). | ||
| if (char_len > data_len - i) { | ||
| gdv_fn_set_error_for_invalid_utf8(context, data[i]); |
There was a problem hiding this comment.
Could we also add NativeFunction::kCanReturnErrors to the lower and upper registrations in function_registry_string.cc, following the same NULL-handling fix as #49035 for binary_string? initcap already has the flag.
Without it, Gandiva invokes these functions even on NULL inputs, so the new truncation checks can fail a batch on bytes belonging to a NULL slot.
Please add Projector-level regressions for both functions, following the setup in TestUtf8.TestInternalAllocs. Construct that NULL input and assert evaluation succeeds and the output remains NULL. Direct stub tests bypass the generated validity handling and cannot catch this.
Fixes #50472 and #50504.
Guard the
int32_toutput-size multiplication inmask_utf8_utf8_utf8_utf8withMultiplyWithOverflow. Also reject incomplete multibyte input before decoding in lower, upper, and initcap, usingchar_len > data_len - iso the bound check cannot itself overflow.Regression coverage includes 65,536-byte input and replacement strings, truncated two-, three-, and four-byte characters across all three converters, and valid full-input controls. Correct the existing InitCap fixture length from 19 to 20 bytes; its final two-byte character was previously outside the declared slice.
Local Linux validation at head
dd47db3afcd06f1b2737bc31250d99a2ee113ec9(tree8a076b9ad09fa1e7f1907b494d7f2ed1580138bb): Clang 18 Debug with ASan and UBSan, all 36TestGdvFnStubstests passed. Against the original source, UBSan detects the signed multiplication overflow and the truncated-input regression fails; restoring the fixes passes all 36 again.These are focused local results. Upstream CI and maintainer review remain separate. This follow-up was assisted by Codex, disclosed in the commit trailer.