Skip to content

query: check the additions a hostile msgpack length drives - #383

Closed
EnRaiha wants to merge 1 commit into
NodeDB-Lab:mainfrom
EnRaiha:fix/msgpack-length-overflow
Closed

EnRaiha wants to merge 1 commit into
NodeDB-Lab:mainfrom
EnRaiha:fix/msgpack-length-overflow

Conversation

@EnRaiha

@EnRaiha EnRaiha commented Sep 27, 2026

Copy link
Copy Markdown
Contributor

query: check the additions a hostile msgpack length drives

Why

The reader module documents "Returns None on truncated/invalid data — never
panics", and four sites broke that contract by adding a length read from the
input to a header width in plain usize:

  • the STR32, BIN32 and EXT32 arms of skip_value
  • start + len in the string read path

5 + 0xffff_ffff fits in a 64-bit usize, so the buffer check rejects it and
the value is discarded as intended. On a 32-bit target the addition overflows
first and the process aborts with attempt to add with overflow — a hostile
frame turning a decode failure into a crash.

What changed

checked_advance_len performs both additions checked, and checked_advance,
read_u16_be, read_u32_be, read_u64_be, read_str, read_str_advance,
read_bin_advance, the field index and the field lookup all route through it.
Every header/len pair was checked against its tag width; none changed value.

Strictly narrower input is rejected. No input that was accepted before is
rejected now.

Steps to test

cargo nextest run -p nodedb-query -E 'test(fuzz_adversarial_length_fields)'

The existing adversarial-length test already covers all four sites. It is the
red arm on a 32-bit target and green on a 64-bit one either side of the change,
which is why the defect survived:

target fix reverted fix applied
wasm32-wasip1 FAILED — attempt to add with overflow PASS
x86_64 PASS PASS

Run it under a 32-bit target to see the failure:

CARGO_TARGET_WASM32_WASIP1_RUNNER="wasmtime -W max-wasm-stack=33554432 --dir=." \
  cargo nextest run --profile ci --target wasm32-wasip1 -p nodedb-query \
  -E 'test(fuzz_adversarial_length_fields)'

The module documents "Returns `None` on truncated/invalid data — never panics",
but four sites added a length read from the input to a header width in plain
`usize`: the STR32, BIN32 and EXT32 arms of `skip_value`, and `start + len` in
the string read path. `5 + 0xffff_ffff` fits in a 64-bit `usize` and the buffer
check rejects it; on a 32-bit target the addition overflows first and the process
aborts.

`checked_advance_len` performs both additions checked, and `checked_advance`,
`read_u16_be`, `read_u32_be`, `read_u64_be`, `read_str`, `read_str_advance`,
`read_bin_advance`, the field index and the field lookup all route through it.
The existing adversarial-length test already covers all four sites; it is
red->green on a 32-bit target and green on both before and after on a 64-bit one,
which is why this went unnoticed:

  wasm32-wasip1, fix reverted: FAILED, "attempt to add with overflow"
  wasm32-wasip1, fix applied:  PASS
  x86_64, either:              PASS
Copilot AI lite review requested due to automatic review settings September 27, 2026 11:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@EnRaiha

EnRaiha commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

Closing: the linked issue is closed. The finding itself is hostile-input hardening on 32-bit usize, not wasm work. If the maintainer wants it, it returns as a fresh PR with that framing.

@EnRaiha EnRaiha closed this Sep 28, 2026
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.

2 participants