Skip to content

fix(arrow-scalar): bound the lengths read while decoding a scalar - #8969

Open
jackylee-ch wants to merge 1 commit into
lance-format:mainfrom
jackylee-ch:fix/scalar-decode-bounds
Open

fix(arrow-scalar): bound the lengths read while decoding a scalar#8969
jackylee-ch wants to merge 1 commit into
lance-format:mainfrom
jackylee-ch:fix/scalar-decode-bounds

Conversation

@jackylee-ch

Copy link
Copy Markdown
Contributor

Three lengths in the scalar encoding come from varints and can reach u64::MAX, and none were checked against the bytes present. ArrowScalar::decode panics on all three:

fmt_len     = u64::MAX -> panic  slice index starts at 10 but ends at 9
num_buffers = u64::MAX -> panic  capacity overflow
buffer_len  = u64::MAX -> panic  slice index starts at 14 but ends at 13

The two length fields computed offset + len, which wraps and so slips past the very comparison meant to bound it. The buffer count was reserved for before anything validated it.

Resolve each run's end through a checked add, and reject a count larger than the remaining bytes.

Three values in the encoding come from varints and reach u64::MAX, and
none of them were checked against the bytes present. The two length
fields computed `offset + len`, which wraps and so slips past the very
comparison meant to bound it, leaving the following slice out of range;
the buffer count was reserved for before anything validated it.

Resolve each run's end through a checked add, and reject a buffer count
larger than the remaining bytes before allocating for it.
@github-actions github-actions Bot added the bug Something isn't working label Sep 3, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Gate recommendation: approve with a non-blocking risk.

The checked endpoints and pre-allocation count bound correctly turn the three reported extreme length/count fields into errors without changing valid encodings.

A pre-existing malformed-type path remains: the self-described null type w:-1 still reaches Arrow’s infallible null allocator and panics. This PR neither introduces nor widens that path, so no change is requested here; follow-up hardening should validate parsed data types before null construction.

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant