diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1a2af8b5..14b37e1d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -57,3 +57,17 @@ jobs: RUST_LOG: debug if: matrix.os == 'windows-latest' run: cargo test --all-features -- --nocapture + + fuzz: + name: Build fuzz targets + runs-on: ubuntu-22.04 + timeout-minutes: 20 + steps: + - uses: actions/checkout@v6 + # libFuzzer needs nightly. + - uses: dtolnay/rust-toolchain@nightly + - name: Install cargo-fuzz + run: cargo install cargo-fuzz --locked + # Compile only, no campaign. `cargo clippy --all-features` already covers src/fuzz_api.rs. + - name: Build fuzz targets + run: cargo fuzz build diff --git a/Cargo.toml b/Cargo.toml index 1df56692..123abe69 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,6 +9,8 @@ repository = "https://github.com/keepsimple1/mdns-sd" documentation = "https://docs.rs/mdns-sd" keywords = ["mdns", "service-discovery", "zeroconf", "dns-sd"] categories = ["network-programming"] +# The fuzz targets are a separate crate and need a nightly toolchain. +exclude = ["fuzz"] description = "mDNS Service Discovery library with no async runtime dependency" [features] @@ -17,6 +19,10 @@ logging = ["log"] serde = ["dep:serde"] default = ["async", "logging"] +# Not part of the public API: exposes crate internals to the fuzz targets under +# `fuzz/`. No stability guarantees; it may change or be removed in any release. +unstable-fuzz-api = [] + [dependencies] fastrand = "2.4" flume = { version = "0.12", default-features = false } # channel between threads diff --git a/fuzz/.gitignore b/fuzz/.gitignore new file mode 100644 index 00000000..1a45eee7 --- /dev/null +++ b/fuzz/.gitignore @@ -0,0 +1,4 @@ +target +corpus +artifacts +coverage diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml new file mode 100644 index 00000000..1d30b3b8 --- /dev/null +++ b/fuzz/Cargo.toml @@ -0,0 +1,28 @@ +[package] +name = "mdns-sd-fuzz" +version = "0.0.0" +publish = false +edition = "2018" + +# An independent workspace: the fuzz targets need a nightly toolchain, while the +# crate under test builds on its MSRV. Keeping them apart means `cargo build` and +# `cargo clippy` at the repo root never try to build the targets. +[workspace] +members = [] + +[package.metadata] +cargo-fuzz = true + +[dependencies] +libfuzzer-sys = "0.4" + +[dependencies.mdns-sd] +path = ".." +features = ["unstable-fuzz-api"] + +[[bin]] +name = "parse_packet" +path = "fuzz_targets/parse_packet.rs" +test = false +doc = false +bench = false diff --git a/fuzz/README.md b/fuzz/README.md new file mode 100644 index 00000000..6ce5765e --- /dev/null +++ b/fuzz/README.md @@ -0,0 +1,54 @@ +# Fuzzing mdns-sd + +The targets here run under [`cargo-fuzz`] (libFuzzer). They are a separate crate +with their own workspace, so `cargo build` and `cargo clippy` at the repo root +never build them, and the crate's MSRV is unaffected. + +## Setup + +```sh +cargo install cargo-fuzz +rustup toolchain install nightly # libFuzzer needs nightly +``` + +## Running + +```sh +cargo +nightly fuzz run parse_packet +``` + +That is the whole command. cargo-fuzz supplies the corpus directory +(`fuzz/corpus/`, created if absent) and the artifact prefix, and the run +continues until interrupted. Pass libFuzzer's own options after a `--`: +`-max_total_time=60` to bound a run, `-jobs=8` to use more cores. + +New inputs accumulate in `fuzz/corpus/`, which is not tracked by git, so later +runs build on whatever earlier ones found. + +## Targets + +| Target | What it covers | +| --- | --- | +| `parse_packet` | `DnsIncoming::new` on raw bytes — the code that `ServiceDaemon` runs on whatever arrives on its UDP socket. | + +## Reaching crate internals + +`dns_parser` is private, so a fuzz target — a separate crate — cannot call it. +The `unstable-fuzz-api` feature exposes [`src/fuzz_api.rs`](../src/fuzz_api.rs), +a `#[doc(hidden)]` module of thin wrappers. It is not public API and carries no +stability guarantee. Targets that need to reach further in (`dns_cache`, say) +should add a wrapper there rather than widening any module's visibility. + +## When a target finds something + +libFuzzer writes the input to `fuzz/artifacts//`. Reproduce it with: + +```sh +cargo +nightly fuzz run parse_packet fuzz/artifacts/parse_packet/crash- +``` + +Once fixed, add a regression test to the crate so the case is covered by +`cargo test` on stable, the way `test_hinfo_char_string_at_end_of_packet` covers +the first crash this target found. + +[`cargo-fuzz`]: https://github.com/rust-fuzz/cargo-fuzz diff --git a/fuzz/fuzz_targets/parse_packet.rs b/fuzz/fuzz_targets/parse_packet.rs new file mode 100644 index 00000000..f68c599a --- /dev/null +++ b/fuzz/fuzz_targets/parse_packet.rs @@ -0,0 +1,20 @@ +//! Fuzzes the mDNS packet parser. +//! +//! `ServiceDaemon` hands `DnsIncoming` the bytes it reads off a UDP socket +//! without inspecting them first, so every byte here is reachable by any host on +//! the link. Malformed input is expected; the target looks for panics, hangs, +//! and self-contradictory results. +//! +//! Run with: +//! +//! ```text +//! cargo +nightly fuzz run parse_packet +//! ``` + +#![no_main] + +use libfuzzer_sys::fuzz_target; + +fuzz_target!(|data: &[u8]| { + mdns_sd::fuzz_api::parse_packet(data); +}); diff --git a/src/fuzz_api.rs b/src/fuzz_api.rs new file mode 100644 index 00000000..ec138511 --- /dev/null +++ b/src/fuzz_api.rs @@ -0,0 +1,72 @@ +//! Entry points for the fuzz targets under `fuzz/`. +//! +//! This module is not part of the public API. It exists because `dns_parser` is +//! private to the crate, and a fuzz target is a separate crate that could not +//! otherwise call it. Keeping thin wrappers here, rather than making that module +//! public, keeps the exposed surface small and deliberate. Targets that need to +//! reach further in (`dns_cache`, say) should add a wrapper here too. + +use crate::dns_parser::{DnsIncoming, InterfaceId}; + +/// The interface a fuzzed packet is attributed to. +/// +/// Records are cached per interface, so the value matters for targets that reach +/// the cache; for parsing alone any fixed value will do. +fn fuzz_interface_id() -> InterfaceId { + InterfaceId { + name: "fuzz0".to_string(), + index: 1, + } +} + +/// Parses one raw packet, the way `ServiceDaemon` parses bytes read off a UDP +/// socket, and checks the invariants that hold for any packet that parses. +/// +/// Malformed input is expected and is not a finding: the target is looking for +/// panics, hangs, and broken invariants, not for parse errors. +pub fn parse_packet(data: &[u8]) { + let msg = match DnsIncoming::new(data.to_vec(), fuzz_interface_id()) { + Ok(msg) => msg, + Err(e) => { + // Render the error as it formats slices of the raw packet, + // so it is worth fuzzing in its own right. + let _ = e.to_string(); + return; + } + }; + + assert_eq!( + msg.questions().len(), + msg.num_questions() as usize, + "parsed question count must match the header" + ); + + // Records, unlike questions, may be skipped individually: a record whose + // rdata does not parse is dropped and the rest of the packet is kept. So + // these counts are upper bounds, not equalities. + // + // The answer section is not checked here only because `DnsIncoming` exposes + // no `num_answers()` getter. + assert!( + msg.authorities().len() <= msg.num_authorities() as usize, + "parsed more authorities than the header declared" + ); + assert!( + msg.additionals().len() <= msg.num_additionals() as usize, + "parsed more additionals than the header declared" + ); + + // A message is exactly one of a query or a response. + assert!( + msg.is_query() != msg.is_response(), + "a message must be either a query or a response" + ); + + // Walk the records, so the fuzzer reaches the accessors and the `Debug` + // impls of the record trait objects rather than stopping at the parse loop. + // `Debug` for `DnsIncoming` slices the raw packet, so exercise it directly. + let _ = format!("{msg:?}"); + for record in msg.all_records() { + let _ = format!("{record:?}"); + } +} diff --git a/src/lib.rs b/src/lib.rs index 3fa155b2..fb98feb0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -177,6 +177,10 @@ mod error; mod service_daemon; mod service_info; +#[cfg(feature = "unstable-fuzz-api")] +#[doc(hidden)] +pub mod fuzz_api; + pub use dns_parser::{InterfaceId, RRType, ScopedIp, ScopedIpV4, ScopedIpV6, MAX_PKT_DEFAULT}; pub use error::{Error, Result}; pub use service_daemon::{