Conversation
An element count read from a frame is multiplied by its element size before the 64 MiB ceiling is compared. On a 32-bit target that multiplication overflows `usize` for counts the 64-bit path rejects cleanly, so the same hostile frame came back as `Corrupt` there and `ResourceLimit` here — and the delta decoder's own test asserts `ResourceLimit`, so the suite aborted on a 32-bit target. `checked_capacity` now does the scaling in 64-bit arithmetic and reports the resource limit whenever the product exceeds the ceiling, on every target. The `requested` field stays a byte count: the branch is only reachable above 64 MiB and below the type's maximum, so the value is always representable. `validate_value_count` in delta and double-delta and the FastLanes header parse route through it rather than repeating the multiplication. On a 64-bit target no reachable input changes: wire counts cap at `u32::MAX`, whose byte size is rejected as `ResourceLimit` by both the old and the new code.
Contributor
Author
|
Reframing: hostile-input hardening on 32-bit usize, not wasm work. NodeDB does not support wasm; this stands or falls on supported targets. Leaving open for the maintainer's call. |
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
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.
codec: scale a decoded element count in 64-bit so 32-bit targets agree
Why
An element count read from a frame is multiplied by its element size before the
64 MiB ceiling is compared. On a 32-bit target that multiplication overflows
usizefor counts the 64-bit path rejects cleanly, so the same hostile framecame back as
Corruptthere andResourceLimithere — and the delta decoder'sown test asserts
ResourceLimit, so the suite aborted on a 32-bit targetinstead of reporting a test failure.
What changed
checked_capacitydoes the scaling in 64-bit arithmetic and reports theresource limit whenever the product exceeds the ceiling, on every target.
validate_value_countin delta and double-delta and the FastLanes header parseroute through it rather than repeating the multiplication.
The
requestedfield stays a byte count: the rejecting branch is only reachableabove the 64 MiB ceiling and below the type's maximum, so the value is always
representable. On a 64-bit target no reachable input changes — wire counts cap
at
u32::MAX, whose byte size both the old and the new code reject asResourceLimit.The classification change is deliberate and was already asserted by the tests:
checked_capacity_rejects_usize_overflowwas replaced by two stricter casescovering
usize::MAXandu32::MAXat element size 8.Steps to test
Exclusions, stated
The eight zstd encoder tests in this crate also fail on
wasm32-wasip1,because that target has no zstd encoder by design (
compress_nativereturnsCompressFailed; it decodes withruzstdand encodes with LZ4). That is adifferent defect in a different test group and is not part of this change.