Skip to content

Commit

Permalink
Fuzz packet decoding (quinn-rs#885)
Browse files Browse the repository at this point in the history
  • Loading branch information
jafow authored Jan 3, 2021
1 parent 9b19e23 commit 9b5d5b3
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 21 deletions.
14 changes: 12 additions & 2 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
- uses: actions-rs/cargo@v1
with:
command: build
args: --workspace --all-targets
args: --all-targets
- uses: actions-rs/cargo@v1
with:
command: test
Expand All @@ -55,7 +55,17 @@ jobs:
if: always()
with:
command: clippy
args: --workspace --all-targets -- -D warnings
args: --all-targets -- -D warnings
- uses: actions-rs/toolchain@v1
with:
profile: minimal
toolchain: stable
override: true
components: clippy
- name: lint fuzz
run: |
cd fuzz
cargo clippy -- -D warnings
audit:
runs-on: ubuntu-latest
Expand Down
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[workspace]
members = ["quinn", "quinn-proto", "quinn-h3", "interop", "bench"]
members = ["quinn", "quinn-proto", "quinn-h3", "interop", "bench", "fuzz"]
default-members = ["quinn", "quinn-proto", "quinn-h3", "interop", "bench"]

[profile.bench]
debug = true
Expand Down
2 changes: 2 additions & 0 deletions fuzz/.cargo/config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[build]
rustflags = "--cfg fuzzing"
19 changes: 12 additions & 7 deletions fuzz/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
[package]
name = "quinn-fuzz"
version = "0.0.0"
authors = ["Automatically generated"]
name = "fuzz"
version = "0.1.0"
authors = [
"Benjamin Saunders <[email protected]>", "Dirkjan Ochtman <[email protected]>",
]
publish = false
license = "MIT OR Apache-2.0"
edition = "2018"

[package.metadata]
Expand All @@ -18,10 +21,6 @@ path = "../quinn-proto"
package = "quinn-proto"
version = "0.6.1"

# Prevent this from interfering with workspaces
[workspace]
members = ["."]

[[bin]]
name = "streams"
path = "fuzz_targets/streams.rs"
Expand All @@ -33,3 +32,9 @@ name = "streamid"
path = "fuzz_targets/streamid.rs"
test = false
doc = false

[[bin]]
name = "packet"
path = "fuzz_targets/packet.rs"
test = false
doc = false
15 changes: 15 additions & 0 deletions fuzz/fuzz_targets/packet.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#![no_main]

use libfuzzer_sys::fuzz_target;
use proto::fuzzing::{PacketParams, PartialDecode};
extern crate proto;

fuzz_target!(|data: PacketParams| {
let len = data.buf.len();
if let Ok(decoded) = PartialDecode::new(data.buf, data.local_cid_len) {
match decoded.1 {
Some(x) => assert_eq!(len, decoded.0.len() + x.len()),
None => assert_eq!(len, decoded.0.len()),
}
}
});
10 changes: 5 additions & 5 deletions fuzz/fuzz_targets/streams.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![no_main]

use arbitrary::{Arbitrary, Unstructured};
use arbitrary::Arbitrary;
use libfuzzer_sys::fuzz_target;

extern crate proto;
Expand All @@ -16,7 +16,7 @@ struct StreamParams {
receive_window: u16,
stream_receive_window: u16,
dir: Dir,
transport_params: TransportParameters
transport_params: TransportParameters,
}

#[derive(Arbitrary, Debug)]
Expand Down Expand Up @@ -49,16 +49,16 @@ fuzz_target!(|input: (StreamParams, Vec<Operation>)| {
stream.accept(dir);
}
Operation::Finish(id) => {
stream.finish(id);
let _ = stream.finish(id);
}
Operation::ReceivedStopSending(sid, err_code) => {
stream.received_stop_sending(sid, err_code);
}
Operation::ReceivedReset(rs) => {
stream.received_reset(rs);
let _ = stream.received_reset(rs);
}
Operation::Reset(id) => {
stream.reset(id);
let _ = stream.reset(id);
}
}
}
Expand Down
23 changes: 21 additions & 2 deletions quinn-proto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
//! related `Connection`. `Connection` types contain the bulk of the protocol logic related to
//! managing a single connection and all the related state (such as streams).
#![warn(missing_docs)]
#![cfg_attr(not(fuzzing), warn(missing_docs))]
#![cfg_attr(test, allow(dead_code))]
// Fixes welcome:
#![allow(clippy::cognitive_complexity)]
Expand Down Expand Up @@ -99,10 +99,13 @@ use arbitrary::Arbitrary;
#[doc(hidden)]
#[cfg(fuzzing)]
pub mod fuzzing {
pub use crate::connection::Streams;
pub use crate::connection::{FinishError, Streams};
pub use crate::frame::ResetStream;
pub use crate::packet::PartialDecode;
pub use crate::transport_parameters::TransportParameters;
use crate::MAX_CID_SIZE;
use arbitrary::{Arbitrary, Result, Unstructured};
pub use bytes::{BufMut, BytesMut};

impl Arbitrary for TransportParameters {
fn arbitrary(u: &mut Unstructured<'_>) -> Result<Self> {
Expand All @@ -115,6 +118,22 @@ pub mod fuzzing {
})
}
}

#[derive(Debug)]
pub struct PacketParams {
pub local_cid_len: usize,
pub buf: BytesMut,
}

impl Arbitrary for PacketParams {
fn arbitrary(u: &mut Unstructured<'_>) -> Result<Self> {
let local_cid_len: usize = u.int_in_range(0..=MAX_CID_SIZE)?;
let bytes: Vec<u8> = Vec::arbitrary(u)?;
let mut buf = BytesMut::new();
buf.put_slice(&bytes[..]);
Ok(PacketParams { local_cid_len, buf })
}
}
}

/// The QUIC protocol version implemented
Expand Down
9 changes: 5 additions & 4 deletions quinn-proto/src/packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ use crate::{
// to inspect the version and packet type (which depends on the version).
// This information allows us to fully decode and decrypt the packet.
#[derive(Debug)]
pub(crate) struct PartialDecode {
pub struct PartialDecode {
plain_header: PlainHeader,
buf: io::Cursor<BytesMut>,
}

impl PartialDecode {
pub(crate) fn new(
#![allow(clippy::len_without_is_empty)]
pub fn new(
bytes: BytesMut,
local_cid_len: usize,
) -> Result<(Self, Option<BytesMut>), PacketDecodeError> {
Expand Down Expand Up @@ -90,7 +91,7 @@ impl PartialDecode {
}

/// Length of QUIC packet being decoded
pub(crate) fn len(&self) -> usize {
pub fn len(&self) -> usize {
self.buf.get_ref().len()
}

Expand Down Expand Up @@ -710,7 +711,7 @@ pub(crate) enum LongType {
}

#[derive(Debug, Error, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
pub(crate) enum PacketDecodeError {
pub enum PacketDecodeError {
#[error("unsupported version {version:x}")]
UnsupportedVersion {
src_cid: ConnectionId,
Expand Down

0 comments on commit 9b5d5b3

Please sign in to comment.