Skip to content

feat(fuzz): add pdu_round_trip oracle and target#1291

Open
Greg Lamberson (glamberson) wants to merge 1 commit into
Devolutions:masterfrom
lamco-admin:feat/pdu-round-trip-oracle
Open

feat(fuzz): add pdu_round_trip oracle and target#1291
Greg Lamberson (glamberson) wants to merge 1 commit into
Devolutions:masterfrom
lamco-admin:feat/pdu-round-trip-oracle

Conversation

@glamberson
Copy link
Copy Markdown
Contributor

Summary

Adds a fuzz oracle and dedicated target for round-trip exercise of PDU
encode/decode pathways. For each PDU type currently covered by
pdu_decode, the oracle attempts decode -> encode_vec -> re-decode
on the encoded bytes. Auto-discovers into CI per #1290.

Property tested

No internal panic from inside the encoder or decoder when fed a
decoder-accepted input through both directions of the round-trip.

Asymmetric Err returns (decoder accepts something the encoder reports
as "Encoding not implemented", or vice-versa) are NOT in scope here:
those are tolerated incomplete-impl cases tracked separately. The
oracle silently drops Err results from any of the three stages.

What this catches:

  • unreachable!() reached during encoding of a valid decoded state
    (i.e. the encoder's match arms are missing a variant the decoder
    produces).
  • Integer overflow / index-out-of-bounds inside the encoder on
    decoder-accepted inputs.
  • Panics in the decoder when fed encoder-produced bytes (re-decode
    path).

Initial type coverage

Mirrors pdu_decode's type list minus two exclusions documented inline
with their reproducers. As new PDU types gain Encode impls, they
extend coverage when added to the macro list.

Excluded pending follow-up

Two types are temporarily excluded because they hit unreachable!() in
the encoder during smoke fuzzing, which is an internal-panic bug that
the oracle cannot silently drop:

  • capability_sets::CapabilitySet: the encoder's match at
    crates/ironrdp-pdu/src/rdp/capability_sets/mod.rs:447 reaches
    unreachable!() on a variant the decoder accepts but the encoder's
    match arms don't cover. Smoke-fuzz reproducer: [6, 0, 4, 0].
  • headers::ShareControlHeader: transits through CapabilitySet's
    encoder for DemandActive / ConfirmActive PDU contents and hits
    the same internal panic.

Both inclusions return once the encoder's match arms are completed. I
will file the underlying bug as a separate issue and link it back here.

Files

  • crates/ironrdp-fuzzing/src/oracles/mod.rs: new pdu_round_trip
    function and pdu_round_trip_one! helper macro.
  • fuzz/fuzz_targets/pdu_round_trip.rs: new fuzz target binary.
  • fuzz/Cargo.toml: [[bin]] entry for the new target.
  • crates/ironrdp-testsuite-core/tests/fuzz_regression.rs: new
    check_pdu_round_trip regression-replay test.
  • crates/ironrdp-testsuite-core/test_data/fuzz_regression/pdu_round_trip/seed-empty.bin:
    empty seed file for the regression-replay test.

Verification

  • cargo xtask check fmt -v green
  • cargo xtask check lints -v green
  • cargo xtask check typos -v green
  • cargo xtask check locks -v green
  • cargo xtask check tests -v green (including
    check_pdu_round_trip)
  • 60-second smoke fuzz: 2,220,481 runs, no crashes on the current
    type set
  • New target auto-discovered by cargo xtask fuzz list (per ci(fuzz): discover fuzz targets dynamically #1290)

Refs #1120 (umbrella) and #1124 (oracle infrastructure).

Adds a fuzz oracle that exercises decode -> encode_vec -> re-decode for
each PDU type currently covered by pdu_decode, plus a dedicated
pdu_round_trip target binary (auto-discovers into CI per Devolutions#1290).

The property tested is *no internal panic from inside the encoder or
decoder when fed a decoder-accepted input through both directions of the
round-trip*. Asymmetric Err returns (decoder accepts something the
encoder reports as "Encoding not implemented", or vice-versa) are not
in scope: those are tolerated incomplete-impl cases tracked separately.
The oracle catches:

- unreachable!() reached during encoding of a valid decoded state
- Integer overflow / index-out-of-bounds inside the encoder on
  decoder-accepted inputs
- Panics in the decoder when fed encoder-produced bytes (re-decode path)

Initial type coverage mirrors pdu_decode minus two known-asymmetric
types that hit `unreachable!()` in the encoder:

- `capability_sets::CapabilitySet`: encoder match at
  crates/ironrdp-pdu/src/rdp/capability_sets/mod.rs:447 reaches
  `unreachable!()` on a variant the decoder accepts but the encoder's
  match arms don't cover. Smoke-fuzz reproducer: `[6, 0, 4, 0]`.

- `headers::ShareControlHeader`: transits through CapabilitySet's
  encoder for DemandActive/ConfirmActive PDU contents and hits the same
  internal panic.

Both are tracked as follow-up filings; the oracle re-includes them
once the encoder's match arms are completed.

Verification:

- cargo xtask check fmt/lints/typos/locks/tests all green
- check_pdu_round_trip regression-replay test passes
- 60-second smoke fuzz: 2,220,481 runs, no crashes on the
  current type set

Refs Devolutions#1120 (umbrella) and Devolutions#1124 (oracle infrastructure).
@glamberson
Copy link
Copy Markdown
Contributor Author

Filed #1292 for the underlying CapabilitySet::BitmapCacheV3 encoder gap. Once that lands, this PR's two excluded types (capability_sets::CapabilitySet and headers::ShareControlHeader) can return to the round-trip coverage list.

Comment thread crates/ironrdp-fuzzing/src/oracles/mod.rs
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!

Small remark: I wonder if this is rendering useless some other harness such as pdu_decode, because we are exercising the decode path here too

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new fuzzing oracle and target to exercise PDU decode/encode round-trips, with a companion regression-replay test to ensure minimized inputs remain covered in CI. This extends the existing fuzz oracle suite in ironrdp-fuzzing and integrates cleanly with the fuzz target discovery CI approach referenced in #1290.

Changes:

  • Add pdu_round_trip oracle that performs decode -> encode_vec -> re-decode, dropping Err results but surfacing internal panics.
  • Add a dedicated pdu_round_trip libFuzzer target and register it in fuzz/Cargo.toml.
  • Add regression-replay coverage via check_pdu_round_trip plus an initial (empty) seed corpus file.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/ironrdp-fuzzing/src/oracles/mod.rs Adds the pdu_round_trip oracle and helper macro, mirroring the pdu_decode type list with documented exclusions.
fuzz/fuzz_targets/pdu_round_trip.rs New fuzz target wrapper calling the oracle on raw input bytes.
fuzz/Cargo.toml Registers the new fuzz target as a [[bin]] so it’s discoverable/buildable.
crates/ironrdp-testsuite-core/tests/fuzz_regression.rs Adds a regression-replay test entry for the new oracle.
crates/ironrdp-testsuite-core/test_data/fuzz_regression/pdu_round_trip/seed-empty.bin Seeds the regression folder so the replay test has at least one input to run.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@glamberson
Copy link
Copy Markdown
Contributor Author

There's real overlap. pdu_round_trip's coverage is technically a superset since its first call exercises the same decoder-panic surface pdu_decode targets. But pdu_decode is still worth keeping for three operational reasons:

  1. Per-iteration cost. pdu_decode does one decode per input; pdu_round_trip does up to three (decode + encode + re-decode). In a fixed-time fuzz run, pdu_decode covers ~3x more inputs against the same decoder surface, so decoder-only bugs surface faster there.

  2. Corpus separation. libFuzzer's corpus distillation runs per-target. pdu_decode's corpus minimizes around decoder-panic-inducing inputs; pdu_round_trip's minimizes around round-trip-fragile inputs. Same fuzzer entropy, different distillation outcomes.

  3. Bug isolation. A pdu_decode crash localizes to the decoder by construction. A pdu_round_trip crash could be in decoder, encoder, or re-decoder; you have to inspect the backtrace. Separate targets keep the first hop of triage cheap.

The type lists also differ slightly: pdu_round_trip excludes capability_sets::CapabilitySet and headers::ShareControlHeader pending #1292's BitmapCacheV3 fix, while pdu_decode keeps them.

This matches the existing workspace pattern too: rle_decompress_bitmap, bitmap_stream, and the codec-specific oracles all share some decoder-path coverage with pdu_decode but exist as separate targets for the same operational reasons.

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.

3 participants