Skip to content

feat(fuzz): add message_decoding_invariants oracle and target#1320

Open
Greg Lamberson (glamberson) wants to merge 1 commit into
Devolutions:masterfrom
lamco-admin:feat/pdu-decode-invariants-fuzz
Open

feat(fuzz): add message_decoding_invariants oracle and target#1320
Greg Lamberson (glamberson) wants to merge 1 commit into
Devolutions:masterfrom
lamco-admin:feat/pdu-decode-invariants-fuzz

Conversation

@glamberson
Copy link
Copy Markdown
Contributor

@glamberson Greg Lamberson (glamberson) commented May 26, 2026

This PR adds an assertion-based oracle that checks the Encode::size()
soundness contract on every decoder-accepted input. The asserted
property is pdu.size() == encode_vec(&pdu).len().

The Encode trait's size() is the contract caller code relies on for
buffer sizing (ensure_size!, cast_length!, downstream allocations).
A size that lies about its own length produces under-allocation
(truncated encode) or over-allocation (wasted memory and
buffer-overflow risk depending on surrounding context). The workspace
already asserts this property against known-good byte fixtures in
ironrdp-testsuite-core/tests/pdu/rdp.rs::buffer_length_is_correct_for_*.
This oracle extends the assertion across the fuzz input surface.

This oracle differs from pdu_round_trip in two ways:

  • pdu_round_trip silently drops Err results. It catches panics
    only.
  • message_decoding_invariants asserts the size contract. A violation
    aborts the fuzz iteration as a libFuzzer crash. The bug-reporting
    mechanism follows the oracle module's documentation.

This oracle catches the following bug classes that pdu_round_trip
does not:

  • Encode::size() returns N but encode writes M, where M does not
    equal N. Callers then over-allocate or under-allocate buffers.
  • The encoder cannot reconstruct decode-acceptable bytes to the same
    length, which means decode loses framing structure.

This oracle does not catch the following bug classes, which other
oracles cover:

  • pdu_decode covers decode-time panics and OOM.
  • pdu_round_trip covers re-decode equality after round-trip via its
    silent-drop pattern. Assertion-based re-decode stays out of scope
    here to keep this oracle's failure mode unambiguous.

Type coverage matches pdu_round_trip. The cohort includes
capability_sets::CapabilitySet and headers::ShareControlHeader.
The latter transits through CapabilitySet's encoder via the Active
variants. The same encoder path exercises both types.

The new target auto-discovers into CI via the cargo xtask fuzz list
dynamic fan-out mechanism.

The test plan covers two checks. First, all five
cargo xtask check fmt/lints/tests/typos/locks gates pass on the
post-rebase head. Second, the new check_message_decoding_invariants
regression-replay test in
crates/ironrdp-testsuite-core/tests/fuzz_regression.rs passes
against the seed corpus.

This PR refs the #1120 fuzzing umbrella. The PR also closes #1124.

Comment thread crates/ironrdp-fuzzing/src/oracles/mod.rs Outdated
Copy link
Copy Markdown
Member

@CBenoit Benoît Cortier (CBenoit) left a comment

Choose a reason for hiding this comment

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

Thank you!

This is looking good to me. My only suggestion here would be to make the patch more "self-contained":

Implements #1124 task 2 (message_decoding_invariants). Companion to pdu_round_trip (#1124 task 1)

Gets much less relevant after the work is merged, and we only care about the final shape of the code

@CBenoit
Copy link
Copy Markdown
Member

#1313 got merged!

@glamberson
Copy link
Copy Markdown
Contributor Author

You're right that the ticket-numbered framing in the doc-comment doesn't earn its space after the PR lands. I stripped the prefix that pointed at #1124 task 2 and #1291 task 1. I rewrote the bug-class contrast that followed it so it stands on its own. I also rewrote the parallel referential framing in the commit-message body for the same reason.

This PR adds an assertion-based oracle that checks the `Encode::size()`
soundness contract on every decoder-accepted input. The asserted
property is `pdu.size() == encode_vec(&pdu).len()`.

The `Encode` trait's `size()` is the contract caller code relies on for
buffer sizing (`ensure_size!`, `cast_length!`, downstream allocations).
A size that lies about its own length produces under-allocation
(truncated encode) or over-allocation (wasted memory and
buffer-overflow risk depending on surrounding context). The workspace
already asserts this property against known-good byte fixtures in
`ironrdp-testsuite-core/tests/pdu/rdp.rs::buffer_length_is_correct_for_*`.
This oracle extends the assertion across the fuzz input surface.

This oracle differs from `pdu_round_trip` in two ways:

- `pdu_round_trip` silently drops `Err` results. It catches panics
  only.
- `message_decoding_invariants` asserts the size contract. A violation
  aborts the fuzz iteration as a libFuzzer crash. The bug-reporting
  mechanism follows the oracle module's documentation.

This oracle catches the following bug classes that `pdu_round_trip`
does not:

- `Encode::size()` returns N but `encode` writes M, where M does not
  equal N. Callers then over-allocate or under-allocate buffers.
- The encoder cannot reconstruct decode-acceptable bytes to the same
  length, which means decode loses framing structure.

This oracle does not catch the following bug classes, which other
oracles cover:

- `pdu_decode` covers decode-time panics and OOM.
- `pdu_round_trip` covers re-decode equality after round-trip via its
  silent-drop pattern. Assertion-based re-decode stays out of scope
  here to keep this oracle's failure mode unambiguous.

Type coverage matches `pdu_round_trip`. The cohort includes
`capability_sets::CapabilitySet` and `headers::ShareControlHeader`.
The latter transits through `CapabilitySet`'s encoder via the Active
variants. The same encoder path exercises both types.

The new target auto-discovers into CI via the `cargo xtask fuzz list`
dynamic fan-out mechanism.

The test plan covers two checks. First, all five
`cargo xtask check fmt/lints/tests/typos/locks` gates pass on the
post-rebase head. Second, the new `check_message_decoding_invariants`
regression-replay test in
`crates/ironrdp-testsuite-core/tests/fuzz_regression.rs` passes
against the seed corpus.

This PR refs the Devolutions#1120 fuzzing umbrella. The PR also closes Devolutions#1124.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Add advanced fuzz targets to verify more invariants (round-trip, framing, state-machine…)

2 participants