Skip to content

fix(precompile): reject truncated and overflowing ABI words in the shielded-pool decoder - #126

Merged
nol4lej merged 1 commit into
mainfrom
security/precompile-abi-decoder-bounds
Aug 8, 2026
Merged

fix(precompile): reject truncated and overflowing ABI words in the shielded-pool decoder#126
nol4lej merged 1 commit into
mainfrom
security/precompile-abi-decoder-bounds

Conversation

@nol4lej

@nol4lej nol4lej commented Aug 8, 2026

Copy link
Copy Markdown
Member

Harden the shielded-pool precompile ABI decoder against truncation and overflow

Every offset, length and count the precompile's ABI decoder reads is attacker-chosen calldata, reachable through a plain eth_call — no signature, no gas spent. Two bug classes let malformed calldata through, and one of them panicked the runtime. This hardens the decoder and reorganizes the crate around the fix.

The vulnerabilities

# Class Mechanism Impact
1 Truncation read_offset / read_length / decode_u32 narrowed uint256 words with low_u32(), keeping only the bottom 32 bits 2^32 + 96 read back as 96; the bounds check validated an offset the sender never wrote and the decoder indexed elsewhere
2 Overflow data_start + length and count * 32 wrapped; the wrapped sum passed the very bounds check meant to reject it (release builds have no overflow-checks) A crafted length built an inverted slice range and panicked the runtime
3 Unbounded alloc Vec::with_capacity(count) reserved from a calldata count before the buffer was known to hold it A declared count of 2^30 reserves gigabytes on input that could never decode

The overflow case was worse than a mis-decode. On a pre-fix build, the wrapping length produced a slice index starts at 128 but ends at 127 panic that aborted execution with a wasm unreachable trap — reachable from an unsigned, gas-free eth_call, i.e. a remote, unauthenticated runtime panic.

The fix

  • A 256-bit word is rejected when it does not fit the type it is read into, never narrowed. word_to_usize checks against usize::MAX (which also keeps 32-bit Wasm and 64-bit native from disagreeing about what's acceptable); decode_u32 refuses anything above u32::MAX.
  • Every offset/length/count arithmetic goes through checked_add / checked_mul / checked_range. Wrapping is impossible; an inverted range can't be constructed.
  • Element spans are bounds-checked before Vec::with_capacity reserves.
  • A .unwrap() on a slice conversion became a propagated error — unreachable given the surrounding checks, but not worth keeping reachable-by-accident in a precompile.

No ABI change. Every selector, parameter and head layout is untouched; well-formed calldata decodes exactly as before. What narrowed is the set of malformed inputs the decoder acts on.

Reorganization

Both files were split into directories by responsibility, no behaviour change:

  • src/abi/mod (doc + re-exports), guard (checked arithmetic), scalar (uint32, bytes32), dynamic (bytes, bytes32[], bytes[])
  • src/dispatch/mod (record_and_dispatch: shared call/gas/result handling + raw dispatch()), origin (from_self / from_caller / unsigned, each building only its origin)

abi/guard is the single place calldata arithmetic and usize conversion live. dispatch/mod holds the call→runtime-call conversion, gas charge and result mapping the three modes shared 90% of. Public API is unchanged — mod.rs re-exports, so calls/ and lib.rs need no edits. Tests moved alongside the code they cover.

Verification

  • 74 unit tests (9 new), clippy clean under the CI feature set.
  • Mutation-tested: reverting word_to_usizelow_u32 breaks three tests; one checked_addwrapping_add breaks another — the direct demonstration that without the check the crafted length passes the bounds check.
  • Differential against two nodes via eth_call: the same truncating and wrapping calldata reaches the pallet on a pre-fix build and is refused at the ABI layer on the fixed one; well-formed calldata still decodes on both. The pre-fix run reproduced the wasm trap; the fixed run shows zero panics.
  • Dev-node adversarial suite (26/26): offsets of 2^32, 2^64, 2^255, uint256::MAX; wrapping lengths; over-wide uint32; truncated heads; a 40-call hostile burst — block height checked after each batch to confirm the node keeps producing.

@nol4lej
nol4lej merged commit 306c5f3 into main Aug 8, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant