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: |
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.
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
marked this pull request as ready for review
September 24, 2026 01:42
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.