GH-50380: [C++][Gandiva] fix out-of-bounds read in byte_substr past end

[Arawoof06]

Rationale for this change

byte_substr_binary_int32_int32 derives startPos from the offset argument but never confirms it falls inside the text. A positive offset larger than text_len leaves startPos >= text_len, so text_len - startPos is negative and that value is stored in *out_len. The following memcpy reads it as a large size_t and runs off the end of both the source text and the text_len-sized output buffer. The offset comes straight from user SQL, so byte_substr(col, n, m) with n past the row length trips it. Under AddressSanitizer, byte_substr("TestString", 10, 15, 10) reports negative-size-param (size=-4) reading 4 bytes past the 10-byte region.

What changes are included in this PR?

Return an empty result when startPos >= text_len, before the length truncation can go negative. The check sits in the callee next to where startPos is computed so every caller is covered.

Are these changes tested?

Yes, TestByteSubstr gains a case with the offset past the end asserting an empty result and no error. The existing cases are unchanged.

Are there any user-facing changes?

No.

This PR contains a "Critical Fix". It fixes an out-of-bounds read (and oversized copy) in byte_substr reachable from user-supplied offsets.

• GitHub Issue: [C++][Gandiva] out-of-bounds read in byte_substr when offset is past the end of the text #50380

[github-actions]

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

[kou]

@dmitry-chirkov-dremio @lriggs @akravchukdremio @xxlaykxx Could you review this?

[Copilot]

Pull request overview

Fixes a Gandiva byte_substr edge case where a user-supplied offset beyond the input length could produce a negative out_len and trigger an out-of-bounds memcpy, and adds regression coverage.

Changes:

• Add a guard in byte_substr_binary_int32_int32 to return an empty result when startPos >= text_len.
• Add a unit test case covering “offset past end” behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
cpp/src/gandiva/precompiled/string_ops.cc	Adds an early-return guard to prevent negative-length truncation and OOB copy in byte_substr.
cpp/src/gandiva/precompiled/string_ops_test.cc	Adds a regression test for offsets past the end returning an empty result.

[Arawoof06]

Good catch. Moved the startPos computation and the past-end check above gdv_fn_context_arena_malloc, so an offset beyond the text now returns empty without allocating or touching the OOM path.

[lriggs]

startPos and length are both int32_t (gdv_int32). After the new early-return guard, startPos is in [0, text_len-1], but length is only required to be > 0 — no upper bound. When length is near INT32_MAX and startPos >= 1, the addition startPos + length overflows signed int32_t to a large negative value. The comparison negative > text_len is false, so the else branch fires: *out_len = length ≈ INT32_MAX. Then memcpy(ret, text + startPos, INT32_MAX) is called against a text_len-byte source and a text_len-byte arena destination — an ~2 GB overread and overwrite on both buffers.

Concrete inputs: text_len=100, offset=2 → startPos=1, length=2147483647 → *out_len = 2147483647 → memcpy(ret, text+1, 2147483647) with 100-byte buffers on both ends.

The same fix that guards the source UTF-8 sibling substr_utf8_int64_int64 applies here: promote to 64-bit before comparing:

if ((int64_t)startPos + length > text_len) {
Or, since startPos < text_len is now guaranteed, even simpler and overflow-free in int32_t:

if (length > text_len - startPos) {