Cintu07 opened a new issue, #11011:
URL: https://github.com/apache/arrow-rs/issues/11011
### Describe the bug
decode_binary_view_inner in arrow-row/src/variable.rs:358 casts a usize
buffer offset down to u32 with nothing bounding it.
views[i] = make_view(val, 0, start_offset as u32);
start_offset is values.len(). values is a single MutableBuffer sized from
values_capacity, which is summed over every long string in the batch, so
nothing caps it. once the decoded data crosses 4 GiB the cast wraps and every
view built after that resolves to the wrong bytes inside the right buffer. no
panic, no error, you get back different strings than you put in. reachable
through decode_binary_view and decode_string_view.
the same crate already guards the i32 version of this. try_into_binary in
arrow-row/src/lib.rs:1464 rejects a buffer over i32::MAX, and the comment under
it says the offsets follow from the length check. the u32 path in the same
crate has neither the check nor an assert.
concat_elements.rs handles it a third way, hoisting the check into
concat_elements_view_array and leaving a debug_assert on the offset, with a
comment that skipping u32::try_from per insertion is worth about 5% in the
benchmark.
### To Reproduce
build Rows over a string column whose long values total more than 4 GiB,
decode with decode_string_view, views past the 4 GiB mark resolve to the wrong
slice. i have not run it, thats 4 GiB of strings and i dont have the machine,
the finding is from reading the code.
### Expected behavior
offsets stay inside u32, or the call fails loudly.
the format already gives the way out. make_view takes a block id and the
decode hardcodes 0, so starting a new buffer when the current one would cross
u32::MAX and bumping the block keeps every offset in range.
coalesce/byte_view.rs already does that with its completed buffers. it touches
a hot loop in more than one place though, so worth a call before anyone writes
it.
### Additional context
same unguarded cast in three other spots,
arrow-cast/src/cast/dictionary.rs:255 and :298
arrow-select/src/coalesce/byte_view.rs:340, three lines under a
try_into().expect("too many buffers") that does check the buffer index
arrow-array/src/array/byte_view_array.rs:1059
happy to take the PR once theres a decision on split versus fail.
--
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]
