Skip to content

GH-50472: [C++][Gandiva] fix integer overflow in mask output allocation#50473

Open
Arawoof06 wants to merge 1 commit into
apache:mainfrom
Arawoof06:mask-output-size-overflow
Open

GH-50472: [C++][Gandiva] fix integer overflow in mask output allocation#50473
Arawoof06 wants to merge 1 commit into
apache:mainfrom
Arawoof06:mask-output-size-overflow

Conversation

@Arawoof06

@Arawoof06 Arawoof06 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

mask_utf8_utf8_utf8_utf8 sizes its output arena buffer with std::max(upper_length, lower_length, num_length) * data_len in int32_t. The replacement lengths and the input length both come from the SQL mask arguments, so a large replacement combined with a large input wraps the multiply and yields a small max_length. The masking loop then writes each replacement past the undersized allocation. With data_len = 65536 and a 65537-byte replacement the true size is 4,295,032,832 which truncates to 65536, and the first masked character already overruns the buffer, crashing with SIGSEGV.

What changes are included in this PR?

Compute the size with arrow::internal::MultiplyWithOverflow and return an error when it overflows, the same guard repeat_utf8_int32, to_hex_binary, and the multibyte translate path already use.

Are these changes tested?

Yes. Added a case to TestGdvFnStubs.TestMask that passes a 64 KiB input with a 64 KiB+1 replacement; it crashes before the change and now returns an error with the fix.

Are there any user-facing changes?

mask on inputs whose masked size would exceed INT32_MAX now returns an error instead of crashing.

This PR contains a "Critical Fix". It fixes a heap buffer overflow (crash) reachable from the mask SQL function on large inputs.

@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #50472 has been automatically assigned in GitHub to PR creator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant