Skip to content

fix(fuzz): handle all reachable EncodeError arms and remove dead MissingReceiver variant#314

Open
ManthanNimodiya wants to merge 1 commit intoopenwallet-foundation-labs:mainfrom
ManthanNimodiya:fix/fuzz-harness-todo-arms
Open

fix(fuzz): handle all reachable EncodeError arms and remove dead MissingReceiver variant#314
ManthanNimodiya wants to merge 1 commit intoopenwallet-foundation-labs:mainfrom
ManthanNimodiya:fix/fuzz-harness-todo-arms

Conversation

@ManthanNimodiya
Copy link
Copy Markdown
Contributor

Summary

The payload_encode_decode fuzz harness had two todo!() arms in its
encode_payload match block. todo!() panics at runtime — when the fuzzer
triggers one of those arms, the harness itself crashes, making it look like
the encoder has a bug when it actually returned a valid error. Additionally,
EncodeError::MissingReceiver was declared in the error enum but never raised
anywhere in the codebase, which obscured the incomplete coverage in the harness.

Root Cause

encode_payload can legitimately return Err(EncodeError::ExcessiveFieldSize)
when any payload field exceeds the CESR variable-data size limit
(3 * 2^24 bytes). The Arbitrary impl in fuzzing.rs generates new_vid
and data fields as unbounded Vec<u8>, so this path is reachable. The outer
_ => todo!() caught it and panicked, producing a false-positive crash report.

The inner todo!() (inside the MissingHops arm) was unreachable in practice
since MissingHops is only raised for RoutedMessage, but it was still
incorrect — unreachable!() or a flat assertion is the right form.

Changes

  • fuzz/fuzz_targets/payload_encode_decode.rs — replaced both todo!()
    arms with explicit handling: ExcessiveFieldSize is silently ignored (it is
    a legitimate rejection), MissingHops is flattened into a single assert!
    with a guard, and a catch-all panic! now surfaces genuinely unexpected errors
    as real fuzz findings
  • tsp_sdk/src/cesr/error.rs — removed MissingReceiver from EncodeError;
    it was never raised by any function in the SDK

Testing

The fuzz target itself is the test. Changes verified by:

  • git grep MissingReceiver returns no results after the removal
  • Diff reviewed against all encode_payload error sites in packet.rs to
    confirm every reachable variant is now explicitly handled

Checklist

  • Follows existing code style
  • No new dependencies
  • No unrelated changes

…ingReceiver variant

Signed-off-by: ManthanNimodiya <manthannimodiya989898@gmail.com>
@ManthanNimodiya ManthanNimodiya force-pushed the fix/fuzz-harness-todo-arms branch from 3232a02 to e9526e0 Compare April 26, 2026 12:29
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