Repository navigation
fix: heap buffer overflow on ndarray shapes whose size overflows - #281
Merged
Merged
Conversation
asdf_ndarray_size and asdf_ndarray_nbytes wrapped, so a file could pass the data size check with a tiny byte count while the conversion loop wrote the true element count into a buffer sized from the wrapped one. Saturate both at UINT64_MAX instead. check_bounds had the same problem with origin + shape, letting a tile or asdf_ndarray_read_at index read outside the array.
cruzzil
force-pushed
the
fix-ndarray-size-overflow
branch
from
October 18, 2026 14:02
27fc575 to
1e40ea7
Compare
Collaborator
|
Technically such a file is not even valid ASDF, as the spec states (somewhat poorly phrased though):
So an ASDF file with such a shape parameter should probably be rejected outright. I'm OK though with accepting it with a warning. Indeed the code should still do something sensible in that case. |
Collaborator
|
LGTM. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
asdf_ndarray_sizeandasdf_ndarray_nbytescan wrap. Withshape: [2**63 + 1]int16 read as int32, the size check sees 2 bytes, allocates 4, andconvert_int16_to_int32writes 2**63+1 elements (ASan heap-buffer-overflow write atndarray_convert.c:409viaasdf_ndarray_read_all). The check from #277 doesn't catch it becausenbyteswraps too.check_boundsalso computesorigin + shape, which wraps: an origin ofUINT64_MAXmadeasdf_ndarray_read_tile_ndimreturnASDF_NDARRAY_OKand read out of bounds. Same forasdf_ndarray_read_at.Fix: both size functions saturate at
UINT64_MAX(a zero dimension still gives 0), andcheck_boundscompares without adding.Tests:
ndarray_read_size_overflow(new fixture) andndarray_read_tile_bounds_overflow; both fail before the fix.AI Disclosure
An LLM detected this whilst reviewing the Rust port for issues, and provided the fix.