diff --git a/.github/workflows/fuzz.yml b/.github/workflows/fuzz.yml new file mode 100644 index 0000000..77c9484 --- /dev/null +++ b/.github/workflows/fuzz.yml @@ -0,0 +1,82 @@ +# WHY(#95): the fuzz targets live outside the workspace and need a nightly +# toolchain, so the stable gate never compiles them. Without this job they would +# be code that is never built and never run — indistinguishable, from the +# outside, from targets that work. This is what makes them evidence. +# +# WHY not on every PR: fuzzing is not a correctness gate for unrelated changes, +# and CI minutes are the fleet's binding constraint at parallelism. It runs when +# the fuzz crate or the code under test changes, weekly to catch drift, and on +# demand. +name: Fuzz + +on: + pull_request: + paths: + - "fuzz/**" + - "crates/kerykeion/**" + - ".github/workflows/fuzz.yml" + schedule: + # Mondays, 05:00 UTC. + - cron: "0 5 * * 1" + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: fuzz-${{ github.event_name == 'push' && github.sha || github.ref }} + cancel-in-progress: true + +jobs: + fuzz: + name: fuzz (${{ matrix.target }}) + runs-on: ubuntu-latest + timeout-minutes: 20 + strategy: + # WHY: one target finding a crash must not cancel the others — the whole + # point is to learn about all three in one run. + fail-fast: false + matrix: + target: [frame_decode, message_parse, routing_decision] + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v6 + with: + persist-credentials: false + + # WHY nightly: cargo-fuzz builds with libFuzzer and a sanitizer, neither of + # which is available on stable. This is scoped to this job; the repository + # toolchain stays stable. + - uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1.17.0 + with: + toolchain: nightly + + - uses: taiki-e/install-action@288e746965032cfcc232e09af2daf5f23c14d780 + with: + tool: cargo-fuzz + + - name: Install protobuf compiler + run: sudo apt-get update && sudo apt-get install -y protobuf-compiler + + # WHY a time box rather than a run count: a smoke test answers "does this + # target build, load its corpus, and execute without crashing", which is a + # duration question. Finding new defects is the scheduled run's job. + # WHY --target is explicit: cargo-fuzz derives its default target from the + # triple *it* was built for, not from the host. install-action ships the + # musl build, so the default became x86_64-unknown-linux-musl — where the + # build fails outright, because a sanitizer cannot be linked against a + # static libc. The runner is gnu; saying so removes the installer's binary + # flavour from the equation. + - name: Fuzz ${{ matrix.target }} + run: | + cargo fuzz run --target x86_64-unknown-linux-gnu ${{ matrix.target }} \ + -- -max_total_time=60 -rss_limit_mb=4096 + + # WHY: libFuzzer writes a reproducer next to the crash. Without this the + # run tells you it failed and destroys the input that proves it. + - name: Upload crash artifacts + if: failure() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v4 + with: + name: fuzz-artifacts-${{ matrix.target }} + path: fuzz/artifacts/ + if-no-files-found: ignore diff --git a/crates/kerykeion/src/codec.rs b/crates/kerykeion/src/codec.rs index cfce63d..48c03c2 100644 --- a/crates/kerykeion/src/codec.rs +++ b/crates/kerykeion/src/codec.rs @@ -25,7 +25,7 @@ use crate::types::{FRAME_MAGIC, MAX_PACKET_SIZE}; /// /// One codec instance is typically wrapped in a [`tokio_util::codec::Framed`] that /// sits on top of a serial port or TCP stream. -pub(crate) struct MeshCodec; +pub struct MeshCodec; impl Decoder for MeshCodec { type Item = FromRadio; diff --git a/fuzz/.gitignore b/fuzz/.gitignore new file mode 100644 index 0000000..735ae0d --- /dev/null +++ b/fuzz/.gitignore @@ -0,0 +1,3 @@ +target +artifacts +coverage diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml new file mode 100644 index 0000000..2c3c8b9 --- /dev/null +++ b/fuzz/Cargo.toml @@ -0,0 +1,48 @@ +# WHY(#95) this is its own workspace: cargo-fuzz targets need a nightly +# toolchain and a sanitizer-instrumented build, while the parent workspace pins +# stable. The empty [workspace] table detaches this crate so `cargo check +# --workspace` and the gate never try to build it, and `members = ["crates/*"]` +# in the root manifest already excludes this directory. + +[package] +name = "kerykeion-fuzz" +version = "0.0.0" +edition = "2024" +rust-version = "1.85" +license = "AGPL-3.0-only" +publish = false + +[package.metadata] +cargo-fuzz = true + +[dependencies] +bytes = "1" +libfuzzer-sys = "0.4" +prost = "0.14.3" +tokio-util = { version = "0.7.18", features = ["codec"] } + +[dependencies.kerykeion] +path = "../crates/kerykeion" + +[[bin]] +name = "frame_decode" +path = "fuzz_targets/frame_decode.rs" +test = false +doc = false +bench = false + +[[bin]] +name = "message_parse" +path = "fuzz_targets/message_parse.rs" +test = false +doc = false +bench = false + +[[bin]] +name = "routing_decision" +path = "fuzz_targets/routing_decision.rs" +test = false +doc = false +bench = false + +[workspace] diff --git a/fuzz/README.md b/fuzz/README.md new file mode 100644 index 0000000..2000874 --- /dev/null +++ b/fuzz/README.md @@ -0,0 +1,66 @@ +# Fuzz targets + +Three libFuzzer targets over the surfaces that parse untrusted radio input, plus the seed corpus they +start from. Written for [`cargo-fuzz`](https://github.com/rust-fuzz/cargo-fuzz). + +## Running them + +Nightly only — libFuzzer and its sanitizers are not on stable. The repository toolchain stays stable; +this crate is deliberately outside the workspace so that stays true. + +```sh +cargo install cargo-fuzz +cargo +nightly fuzz run frame_decode # until you stop it +cargo +nightly fuzz run frame_decode -- -max_total_time=60 # bounded, as CI runs it +cargo +nightly fuzz list +``` + +If `cargo-fuzz` was installed as a musl binary, pass the host triple explicitly — it takes its +default build target from the triple it was built for, not from the machine it runs on, and a +sanitizer cannot link against a static libc: + +```sh +cargo +nightly fuzz run --target x86_64-unknown-linux-gnu frame_decode +``` + +A crash writes its reproducer to `fuzz/artifacts//`. Re-run one with: + +```sh +cargo +nightly fuzz run frame_decode fuzz/artifacts/frame_decode/crash- +``` + +## The targets + +| Target | Surface | Property | +|---|---|---| +| `frame_decode` | `codec::MeshCodec` | never panics; a yielded frame consumed input | +| `message_parse` | `FromRadio` / `ToRadio` | decoding fails cleanly; re-encoding reaches a fixed point | +| `routing_decision` | `RoutingProcessor::process_routing` | every decodable packet gets a verdict | + +`frame_decode` asserts progress rather than only absence of panic: a decoder that reports a frame +without consuming bytes turns `Framed`'s loop into a hang, which a panic-only check would not see. + +`message_parse` asserts that encoding reaches a fixed point — a second pass produces the same bytes as +the first — so a parser accepting more than the type can represent shows up as a field that survives +one round and not two. + +It compares **bytes**, not decoded values, and that distinction was found rather than designed. These +messages carry `f32` fields and `NaN != NaN`, so a value comparison reported a byte-perfect round trip +of a NaN `NodeInfo.snr` as a failure. The fuzzer produced it within a minute of the target first +running; `corpus/message_parse/nan_snr.bin` is that exact input. + +## The corpus + +`corpus//` holds hand-built seeds rather than captured traffic, each one a shape named for +what it exercises: valid frames, a frame split across the magic pair, a length field exceeding +`MAX_PACKET_SIZE`, a truncated varint, an unknown field number. The bytes are derived from the frame +layout documented at the top of `crates/kerykeion/src/codec.rs` and from protobuf wire encoding. + +libFuzzer grows this corpus as it finds new coverage. Committing an input it discovers — particularly +one that reproduced a defect — is how a fixed bug stays fixed. + +## CI + +`.github/workflows/fuzz.yml` runs each target for 60 seconds on changes to this crate or to +`crates/kerykeion`, weekly, and on demand. That is a smoke test: it proves the targets build, load +their corpus and execute. Finding new defects is the scheduled run's job, and a longer local run's. diff --git a/fuzz/corpus/frame_decode/leading_garbage.bin b/fuzz/corpus/frame_decode/leading_garbage.bin new file mode 100644 index 0000000..979d0d7 Binary files /dev/null and b/fuzz/corpus/frame_decode/leading_garbage.bin differ diff --git a/fuzz/corpus/frame_decode/length_exceeds_max.bin b/fuzz/corpus/frame_decode/length_exceeds_max.bin new file mode 100644 index 0000000..3b1a07a --- /dev/null +++ b/fuzz/corpus/frame_decode/length_exceeds_max.bin @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/fuzz/corpus/frame_decode/length_without_body.bin b/fuzz/corpus/frame_decode/length_without_body.bin new file mode 100644 index 0000000..565d92e Binary files /dev/null and b/fuzz/corpus/frame_decode/length_without_body.bin differ diff --git a/fuzz/corpus/frame_decode/lone_magic_byte.bin b/fuzz/corpus/frame_decode/lone_magic_byte.bin new file mode 100644 index 0000000..16e45d3 --- /dev/null +++ b/fuzz/corpus/frame_decode/lone_magic_byte.bin @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/fuzz/corpus/frame_decode/no_magic.bin b/fuzz/corpus/frame_decode/no_magic.bin new file mode 100644 index 0000000..7d174b1 --- /dev/null +++ b/fuzz/corpus/frame_decode/no_magic.bin @@ -0,0 +1 @@ +ޭ \ No newline at end of file diff --git a/fuzz/corpus/frame_decode/truncated_header.bin b/fuzz/corpus/frame_decode/truncated_header.bin new file mode 100644 index 0000000..b21dfe8 Binary files /dev/null and b/fuzz/corpus/frame_decode/truncated_header.bin differ diff --git a/fuzz/corpus/frame_decode/two_frames.bin b/fuzz/corpus/frame_decode/two_frames.bin new file mode 100644 index 0000000..2a9944d Binary files /dev/null and b/fuzz/corpus/frame_decode/two_frames.bin differ diff --git a/fuzz/corpus/frame_decode/valid_empty_payload.bin b/fuzz/corpus/frame_decode/valid_empty_payload.bin new file mode 100644 index 0000000..41bdf1d Binary files /dev/null and b/fuzz/corpus/frame_decode/valid_empty_payload.bin differ diff --git a/fuzz/corpus/frame_decode/valid_id_frame.bin b/fuzz/corpus/frame_decode/valid_id_frame.bin new file mode 100644 index 0000000..24d96b6 Binary files /dev/null and b/fuzz/corpus/frame_decode/valid_id_frame.bin differ diff --git a/fuzz/corpus/message_parse/empty.bin b/fuzz/corpus/message_parse/empty.bin new file mode 100644 index 0000000..e69de29 diff --git a/fuzz/corpus/message_parse/id_only.bin b/fuzz/corpus/message_parse/id_only.bin new file mode 100644 index 0000000..cd3e63a --- /dev/null +++ b/fuzz/corpus/message_parse/id_only.bin @@ -0,0 +1 @@ +* \ No newline at end of file diff --git a/fuzz/corpus/message_parse/length_delimited.bin b/fuzz/corpus/message_parse/length_delimited.bin new file mode 100644 index 0000000..4baf8e1 --- /dev/null +++ b/fuzz/corpus/message_parse/length_delimited.bin @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/fuzz/corpus/message_parse/nan_snr.bin b/fuzz/corpus/message_parse/nan_snr.bin new file mode 100644 index 0000000..b87d8f6 --- /dev/null +++ b/fuzz/corpus/message_parse/nan_snr.bin @@ -0,0 +1 @@ +"%ԍ \ No newline at end of file diff --git a/fuzz/corpus/message_parse/overlong_length.bin b/fuzz/corpus/message_parse/overlong_length.bin new file mode 100644 index 0000000..04d63b0 --- /dev/null +++ b/fuzz/corpus/message_parse/overlong_length.bin @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/fuzz/corpus/message_parse/truncated_varint.bin b/fuzz/corpus/message_parse/truncated_varint.bin new file mode 100644 index 0000000..5a77f05 --- /dev/null +++ b/fuzz/corpus/message_parse/truncated_varint.bin @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/fuzz/corpus/message_parse/unknown_field.bin b/fuzz/corpus/message_parse/unknown_field.bin new file mode 100644 index 0000000..25fcc20 --- /dev/null +++ b/fuzz/corpus/message_parse/unknown_field.bin @@ -0,0 +1 @@ +@ \ No newline at end of file diff --git a/fuzz/corpus/routing_decision/empty.bin b/fuzz/corpus/routing_decision/empty.bin new file mode 100644 index 0000000..e69de29 diff --git a/fuzz/corpus/routing_decision/minimal_packet.bin b/fuzz/corpus/routing_decision/minimal_packet.bin new file mode 100644 index 0000000..e19a122 --- /dev/null +++ b/fuzz/corpus/routing_decision/minimal_packet.bin @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/fuzz/corpus/routing_decision/nested_payload.bin b/fuzz/corpus/routing_decision/nested_payload.bin new file mode 100644 index 0000000..4baf8e1 --- /dev/null +++ b/fuzz/corpus/routing_decision/nested_payload.bin @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/fuzz/corpus/routing_decision/unknown_field.bin b/fuzz/corpus/routing_decision/unknown_field.bin new file mode 100644 index 0000000..25fcc20 --- /dev/null +++ b/fuzz/corpus/routing_decision/unknown_field.bin @@ -0,0 +1 @@ +@ \ No newline at end of file diff --git a/fuzz/fuzz_targets/frame_decode.rs b/fuzz/fuzz_targets/frame_decode.rs new file mode 100644 index 0000000..1bd30b0 --- /dev/null +++ b/fuzz/fuzz_targets/frame_decode.rs @@ -0,0 +1,42 @@ +//! Frame decoding against arbitrary bytes. +//! +//! The codec reads the 4-byte Meshtastic header off a radio link, so every byte +//! it sees is attacker-reachable. Its contract is that it either yields a frame, +//! asks for more input, or errors — never panics, and never spins without +//! consuming input. + +#![no_main] + +use bytes::BytesMut; +use kerykeion::codec::MeshCodec; +use libfuzzer_sys::fuzz_target; +use tokio_util::codec::Decoder; + +/// Ceiling on decode calls per input. +/// +/// WHY: `Framed` drives the decoder in a loop, so the interesting behaviour is a +/// sequence of decodes over one buffer rather than a single call. The bound +/// keeps a pathological input from turning one case into an unbounded loop; a +/// decoder that genuinely never terminates still trips libFuzzer's own timeout, +/// so this hides nothing. +const MAX_DECODES: usize = 4096; + +fuzz_target!(|data: &[u8]| { + let mut codec = MeshCodec; + let mut buf = BytesMut::from(data); + + for _ in 0..MAX_DECODES { + let before = buf.len(); + match codec.decode(&mut buf) { + Ok(Some(_frame)) => { + // A yielded frame must have consumed input. Standing still while + // reporting progress is how a decode loop becomes a hang. + assert!( + buf.len() < before, + "decode yielded a frame without consuming input" + ); + } + Ok(None) | Err(_) => break, + } + } +}); diff --git a/fuzz/fuzz_targets/message_parse.rs b/fuzz/fuzz_targets/message_parse.rs new file mode 100644 index 0000000..a810ef3 --- /dev/null +++ b/fuzz/fuzz_targets/message_parse.rs @@ -0,0 +1,36 @@ +//! Protobuf message parsing against arbitrary bytes. +//! +//! Both directions of the session protocol are decoded from bytes that arrived +//! over the radio. Decoding must fail cleanly rather than panic, and anything +//! that decodes must survive being re-encoded. + +#![no_main] + +use kerykeion::proto::{FromRadio, ToRadio}; +use libfuzzer_sys::fuzz_target; +use prost::Message as _; + +fuzz_target!(|data: &[u8]| { + if let Ok(message) = FromRadio::decode(data) { + // A message the parser accepted must be representable again, and + // encoding must reach a fixed point: a second pass has to produce the + // same bytes as the first. A parser that admits more than the type + // models shows up here as a field that survives one round and not two. + // + // WHY compare bytes rather than the decoded values: these messages carry + // f32 fields, and `NaN != NaN`. Comparing values reports a byte-perfect + // round trip of a NaN as a failure — which this target did, on + // `NodeInfo.snr` (`22 05 25 d4 8d ff ff`, kept as a corpus seed), within + // a minute of first running. Bytes are the property that was meant. + let once = message.encode_to_vec(); + let again = FromRadio::decode(once.as_slice()) + .expect("a message this parser produced must decode again") + .encode_to_vec(); + assert_eq!(once, again, "re-encoding must reach a fixed point"); + } + + // The outbound direction is parsed from untrusted input too, in the gateway + // and replay paths. No round-trip assertion here: only the inbound type is + // reconstructed from the wire in normal operation. + let _ = ToRadio::decode(data); +}); diff --git a/fuzz/fuzz_targets/routing_decision.rs b/fuzz/fuzz_targets/routing_decision.rs new file mode 100644 index 0000000..cff786a --- /dev/null +++ b/fuzz/fuzz_targets/routing_decision.rs @@ -0,0 +1,21 @@ +//! Routing decisions over arbitrary decoded packets. +//! +//! Routing acts on packets the mesh supplies, so a hostile neighbour chooses the +//! hop counts, ports and payload shapes this sees. The decision must be total: +//! every packet that decodes produces a verdict rather than a panic. + +#![no_main] + +use kerykeion::processor::RoutingProcessor; +use kerykeion::proto::MeshPacket; +use libfuzzer_sys::fuzz_target; +use prost::Message as _; + +fuzz_target!(|data: &[u8]| { + // Only well-formed packets reach routing; malformed bytes are the + // message_parse target's subject, and feeding them here would spend the + // budget re-testing the parser instead of the decision. + if let Ok(packet) = MeshPacket::decode(data) { + let _verdict = RoutingProcessor::process_routing(&packet); + } +});