Skip to content

[57_maintenance] Prevent FixedSizeBinaryArray i32 offset overflows (#9872)#9928

Merged
alamb merged 1 commit intoapache:57_maintenancefrom
alamb:alamb/backport_9872_57
May 6, 2026
Merged

[57_maintenance] Prevent FixedSizeBinaryArray i32 offset overflows (#9872)#9928
alamb merged 1 commit intoapache:57_maintenancefrom
alamb:alamb/backport_9872_57

Conversation

@alamb
Copy link
Copy Markdown
Contributor

@alamb alamb commented May 5, 2026

…#9872)

- Closes apache#9850

`FixedSizeBinaryArray::value_offset_at` works use `i32` arithmetic which
can overflow. For offsets beyond `i32::MAX`, that can be bad

1. Prevent any FixedSizedBinaryArrays from being constructed where the
offset calculation could overflow
2. Add some other overflow checks

As @adamreeve [pointed
out](apache#9850 (comment))
on apache#9850 there are several places
where the `i32` arithmetic is problematic in `FixedSizeBinaryArray`. I
will fix them for real in a different, follow on PR, by switching to
entirely `usize` based arithmetic for offset calculations

However, since I hope to backport this PR to older releases, I would
like something that is easy to review and has the least potential for
unintended consequences.

I added unit tests. However, I can't find any way to fully trigger the
actual paths short of trying to allocate very large arrays, which I
don't think is appropriate for unit tests.

Better limit checking
@github-actions github-actions Bot added the arrow Changes to the arrow crate label May 5, 2026
@alamb alamb marked this pull request as ready for review May 5, 2026 20:30
Copy link
Copy Markdown
Contributor

@etseidl etseidl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly good 👍

@alamb alamb merged commit 315e69f into apache:57_maintenance May 6, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrow Changes to the arrow crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants