Arawoof06 opened a new pull request, #50381:
URL: https://github.com/apache/arrow/pull/50381
### 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
