From fa53d9556bb141529729c00bc695881a7363937e Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Fri, 9 Jun 2017 12:06:38 -0700 Subject: [PATCH] Fuzz HPACK --- Cargo.toml | 2 + src/hpack/encoder.rs | 171 +++++++++++++---- src/hpack/huffman/mod.rs | 17 +- src/hpack/mod.rs | 3 +- src/hpack/table.rs | 32 +++- src/hpack/test.rs | 398 ++++++++++++++++++++++++++++++++++++++- 6 files changed, 568 insertions(+), 55 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index bc7cf20..bfb011c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,3 +18,5 @@ hex = "0.2.0" walkdir = "1.0.0" serde = "1.0.0" serde_json = "1.0.0" +quickcheck = "0.4.1" +rand = "0.3.15" diff --git a/src/hpack/encoder.rs b/src/hpack/encoder.rs index 24c186a..b8d0b3e 100644 --- a/src/hpack/encoder.rs +++ b/src/hpack/encoder.rs @@ -9,8 +9,14 @@ pub struct Encoder { size_update: Option, } -#[derive(Debug)] +pub enum Encode { + Full, + Partial(Index), +} + +#[derive(Debug, PartialEq, Eq)] pub enum EncoderError { + BufferOverflow, } #[derive(Debug, Copy, Clone, Eq, PartialEq)] @@ -60,68 +66,125 @@ impl Encoder { } } - pub fn encode<'a, I>(&mut self, headers: I, dst: &mut BytesMut) -> Result<(), EncoderError> - where I: IntoIterator, + /// Encode a set of headers into the provide buffer + pub fn encode(&mut self, resume: Option, headers: &mut I, dst: &mut BytesMut) + -> Result + where I: Iterator, { + let len = dst.len(); + + if let Err(e) = self.encode_size_updates(dst) { + if e == EncoderError::BufferOverflow { + dst.truncate(len); + } + + return Err(e); + } + + if let Some(index) = resume { + let len = dst.len(); + + match self.encode_header(&index, dst) { + Err(EncoderError::BufferOverflow) => { + dst.truncate(len); + return Ok(Encode::Partial(index)); + } + Err(e) => return Err(e), + Ok(_) => {} + } + } + + for header in headers { + let len = dst.len(); + let index = self.table.index(header); + + match self.encode_header(&index, dst) { + Err(EncoderError::BufferOverflow) => { + dst.truncate(len); + return Ok(Encode::Partial(index)); + } + Err(e) => return Err(e), + Ok(_) => {} + } + } + + Ok(Encode::Full) + } + + fn encode_size_updates(&mut self, dst: &mut BytesMut) -> Result<(), EncoderError> { match self.size_update.take() { Some(SizeUpdate::One(val)) => { self.table.resize(val); - encode_size_update(val, dst); + try!(encode_size_update(val, dst)); } Some(SizeUpdate::Two(min, max)) => { self.table.resize(min); self.table.resize(max); - encode_size_update(min, dst); - encode_size_update(max, dst); + try!(encode_size_update(min, dst)); + try!(encode_size_update(max, dst)); } None => {} } - for h in headers { - try!(self.encode_header(h, dst)); - } - Ok(()) } - fn encode_header(&mut self, header: Header, dst: &mut BytesMut) + fn encode_header(&mut self, index: &Index, dst: &mut BytesMut) -> Result<(), EncoderError> { - match self.table.index(header) { - Index::Indexed(idx, header) => { - assert!(!header.is_sensitive()); - encode_int(idx, 7, 0x80, dst); + match *index { + Index::Indexed(idx, _) => { + let header = self.table.resolve(&index); + try!(encode_int(idx, 7, 0x80, dst)); } - Index::Name(idx, header) => { + Index::Name(idx, _) => { + let header = self.table.resolve(&index); + if header.is_sensitive() { - encode_int(idx, 4, 0b10000, dst); + try!(encode_int(idx, 4, 0b10000, dst)); } else { - encode_int(idx, 4, 0, dst); + try!(encode_int(idx, 4, 0, dst)); } - encode_str(header.value_slice(), dst); + try!(encode_str(header.value_slice(), dst)); } - Index::Inserted(header) => { - assert!(!header.is_sensitive()); - dst.put_u8(0b01000000); - encode_str(header.name().as_slice(), dst); - encode_str(header.value_slice(), dst); - } - Index::InsertedValue(idx, header) => { + Index::Inserted(idx) => { + let header = self.table.resolve(&index); + assert!(!header.is_sensitive()); - encode_int(idx, 6, 0b01000000, dst); - encode_str(header.value_slice(), dst); + if !dst.has_remaining_mut() { + return Err(EncoderError::BufferOverflow); + } + + dst.put_u8(0b01000000); + + try!(encode_str(header.name().as_slice(), dst)); + try!(encode_str(header.value_slice(), dst)); } - Index::NotIndexed(header) => { + Index::InsertedValue(idx, _) => { + let header = self.table.resolve(&index); + + assert!(!header.is_sensitive()); + + try!(encode_int(idx, 6, 0b01000000, dst)); + try!(encode_str(header.value_slice(), dst)); + } + Index::NotIndexed(_) => { + let header = self.table.resolve(&index); + + if !dst.has_remaining_mut() { + return Err(EncoderError::BufferOverflow); + } + if header.is_sensitive() { dst.put_u8(0b10000); } else { dst.put_u8(0); } - encode_str(header.name().as_slice(), dst); - encode_str(header.value_slice(), dst); + try!(encode_str(header.name().as_slice(), dst)); + try!(encode_str(header.value_slice(), dst)); } } @@ -135,9 +198,13 @@ impl Default for Encoder { } } -fn encode_str(val: &[u8], dst: &mut BytesMut) { +fn encode_str(val: &[u8], dst: &mut BytesMut) -> Result<(), EncoderError> { use std::io::Cursor; + if !dst.has_remaining_mut() { + return Err(EncoderError::BufferOverflow); + } + if val.len() != 0 { let idx = dst.len(); @@ -145,7 +212,7 @@ fn encode_str(val: &[u8], dst: &mut BytesMut) { dst.put_u8(0); // Encode with huffman - huffman::encode(val, dst); + try!(huffman::encode(val, dst)); let huff_len = dst.len() - (idx + 1); @@ -162,6 +229,10 @@ fn encode_str(val: &[u8], dst: &mut BytesMut) { head_dst.position() as usize }; + if dst.remaining_mut() < head_len { + return Err(EncoderError::BufferOverflow); + } + // This is just done to reserve space in the destination dst.put_slice(&buf[1..head_len]); @@ -181,9 +252,11 @@ fn encode_str(val: &[u8], dst: &mut BytesMut) { // Write an empty string dst.put_u8(0); } + + Ok(()) } -fn encode_size_update(val: usize, dst: &mut B) { +fn encode_size_update(val: usize, dst: &mut B) -> Result<(), EncoderError> { encode_int(val, 5, 0b00100000, dst) } @@ -193,10 +266,17 @@ fn encode_int( prefix_bits: usize, // The number of bits in the prefix first_byte: u8, // The base upon which to start encoding the int dst: &mut B) // The destination buffer + -> Result<(), EncoderError> { + let mut rem = dst.remaining_mut(); + + if rem == 0 { + return Err(EncoderError::BufferOverflow); + } + if encode_int_one_byte(value, prefix_bits) { dst.put_u8(first_byte | value as u8); - return; + return Ok(()); } let low = (1 << prefix_bits) - 1; @@ -208,13 +288,27 @@ fn encode_int( } dst.put_u8(first_byte | low as u8); + rem -= 1; while value >= 128 { + if rem == 0 { + return Err(EncoderError::BufferOverflow); + } + dst.put_u8(0b10000000 | value as u8); + rem -= 1; + value = value >> 7; } + if rem == 0 { + return Err(EncoderError::BufferOverflow); + } + dst.put_u8(value as u8); + rem -= 1; + + Ok(()) } /// Returns true if the in the int can be fully encoded in the first byte. @@ -545,11 +639,6 @@ mod test { assert_eq!(&[32 | 31, 69, 0x80 | 62], &res[..]); } - #[test] - fn test_encoding_into_undersized_buf() { - // Test hitting end at multiple points. - } - #[test] fn test_evicted_overflow() { // Not sure what the best way to do this is. @@ -557,7 +646,7 @@ mod test { fn encode(e: &mut Encoder, hdrs: Vec
) -> BytesMut { let mut dst = BytesMut::with_capacity(1024); - e.encode(hdrs, &mut dst); + e.encode(None, &mut hdrs.into_iter(), &mut dst); dst } diff --git a/src/hpack/huffman/mod.rs b/src/hpack/huffman/mod.rs index c11dd48..9c172f0 100644 --- a/src/hpack/huffman/mod.rs +++ b/src/hpack/huffman/mod.rs @@ -1,7 +1,7 @@ mod table; use self::table::{ENCODE_TABLE, DECODE_TABLE}; -use hpack::DecoderError; +use hpack::{DecoderError, EncoderError}; use bytes::{BytesMut, BufMut}; @@ -43,9 +43,10 @@ pub fn decode(src: &[u8]) -> Result { } // TODO: return error when there is not enough room to encode the value -pub fn encode(src: &[u8], dst: &mut B) { +pub fn encode(src: &[u8], dst: &mut B) -> Result<(), EncoderError> { let mut bits: u64 = 0; let mut bits_left = 40; + let mut rem = dst.remaining_mut(); for &b in src { let (nbits, code) = ENCODE_TABLE[b as usize]; @@ -54,17 +55,29 @@ pub fn encode(src: &[u8], dst: &mut B) { bits_left -= nbits; while (bits_left <= 32) { + if rem == 0 { + return Err(EncoderError::BufferOverflow); + } + dst.put_u8((bits >> 32) as u8); + bits <<= 8; bits_left += 8; + rem -= 1; } } if bits_left != 40 { + if rem == 0 { + return Err(EncoderError::BufferOverflow); + } + // This writes the EOS token bits |= (1 << bits_left) - 1; dst.put_u8((bits >> 32) as u8); } + + Ok(()) } impl Decoder { diff --git a/src/hpack/mod.rs b/src/hpack/mod.rs index 16aae13..ed4aa98 100644 --- a/src/hpack/mod.rs +++ b/src/hpack/mod.rs @@ -7,6 +7,7 @@ mod table; #[cfg(test)] mod test; -pub use self::encoder::Encoder; +pub use self::encoder::{Encoder, Encode, EncoderError}; pub use self::header::Header; pub use self::decoder::{Decoder, DecoderError}; +pub use self::table::Index; diff --git a/src/hpack/table.rs b/src/hpack/table.rs index cd19393..fe701a8 100644 --- a/src/hpack/table.rs +++ b/src/hpack/table.rs @@ -8,6 +8,7 @@ use std::{cmp, mem, usize}; use std::collections::VecDeque; use std::hash::{Hash, Hasher}; +/// HPACK encoder table pub struct Table { mask: usize, indices: Vec>, @@ -19,7 +20,7 @@ pub struct Table { } #[derive(Debug)] -pub enum Index<'a> { +pub enum Index { // The header is already fully indexed Indexed(usize, Header), @@ -27,10 +28,10 @@ pub enum Index<'a> { Name(usize, Header), // The full header has been inserted into the table. - Inserted(&'a Header), + Inserted(usize), - // Only the value has been inserted - InsertedValue(usize, &'a Header), + // Only the value has been inserted (hpack table idx, slots idx) + InsertedValue(usize, usize), // The header is not indexed by this table NotIndexed(Header), @@ -104,6 +105,19 @@ impl Table { self.max_size } + /// Gets the header stored in the table + pub fn resolve<'a>(&'a self, index: &'a Index) -> &'a Header { + use self::Index::*; + + match *index { + Indexed(_, ref h) => h, + Name(_, ref h) => h, + Inserted(idx) => &self.slots[idx].header, + InsertedValue(_, idx) => &self.slots[idx].header, + NotIndexed(ref h) => h, + } + } + /// Index the header in the HPACK table. pub fn index(&mut self, header: Header) -> Index { // Check the static table @@ -219,7 +233,7 @@ impl Table { // Even if the previous header was evicted, we can still reference // it when inserting the new one... - return Index::InsertedValue(real_idx + DYN_OFFSET, &self.slots[0].header); + return Index::InsertedValue(real_idx + DYN_OFFSET, 0); } Index::NotIndexed(header) @@ -279,9 +293,9 @@ impl Table { } if let Some((n, _)) = statik { - Index::InsertedValue(n, &self.slots[0].header) + Index::InsertedValue(n, 0) } else { - Index::Inserted(&self.slots[0].header) + Index::Inserted(0) } } @@ -455,8 +469,8 @@ impl Table { } } -impl<'a> Index<'a> { - fn new(v: Option<(usize, bool)>, e: Header) -> Index<'a> { +impl Index { + fn new(v: Option<(usize, bool)>, e: Header) -> Index { match v { None => Index::NotIndexed(e), Some((n, true)) => Index::Indexed(n, e), diff --git a/src/hpack/test.rs b/src/hpack/test.rs index 94e09b1..af0c255 100644 --- a/src/hpack/test.rs +++ b/src/hpack/test.rs @@ -3,18 +3,412 @@ extern crate hex; extern crate walkdir; extern crate serde; extern crate serde_json; +extern crate quickcheck; +extern crate rand; -use super::{Header, Decoder, Encoder}; +use super::{Header, Decoder, Encoder, Encode}; + +use http::header::{HeaderName, HeaderValue}; use self::bytes::BytesMut; use self::hex::FromHex; use self::serde_json::Value; use self::walkdir::WalkDir; +use self::quickcheck::{QuickCheck, Arbitrary, Gen, TestResult}; +use self::rand::{StdRng, Rng, SeedableRng}; use std::env; use std::fs::File; use std::io::prelude::*; use std::path::Path; +use std::str; + +const MAX_CHUNK: usize = 2 * 1024; + +#[test] +fn hpack_fuzz_failing_1() { + FuzzHpack::new_reduced([ + 12433181738898983662, + 14102727336666980714, + 6105092270172216412, + 16258270543720336235, + ], 1).run(); +} + +#[test] +fn hpack_fuzz() { + fn prop(fuzz: FuzzHpack) -> TestResult { + fuzz.run(); + TestResult::from_bool(true) + } + + QuickCheck::new() + .tests(5000) + .quickcheck(prop as fn(FuzzHpack) -> TestResult) +} + +#[derive(Debug, Clone)] +struct FuzzHpack { + // The magic seed that makes the test case reproducible + seed: [usize; 4], + + // The set of headers to encode / decode + headers: Vec
, + + // The list of chunk sizes to do it in + chunks: Vec, + + // Number of times reduced + reduced: usize, +} + +impl FuzzHpack { + fn new_reduced(seed: [usize; 4], i: usize) -> FuzzHpack { + let mut ret = FuzzHpack::new(seed); + ret.headers.drain(i..); + ret + } + + fn new(seed: [usize; 4]) -> FuzzHpack { + // Seed the RNG + let mut rng = StdRng::from_seed(&seed); + + // Generates a bunch of source headers + let mut source: Vec
= vec![]; + + for _ in 0..2000 { + source.push(gen_header(&mut rng)); + } + + // Actual test run headers + let num: usize = rng.gen_range(40, 300); + let mut actual: Vec
= vec![]; + + let skew: i32 = rng.gen_range(1, 5); + + while actual.len() < num { + let x: f64 = rng.gen_range(0.0, 1.0); + let x = x.powi(skew); + + let i = (x * source.len() as f64) as usize; + actual.push(source[i].clone()); + } + + // Now, generate the buffer sizes used to encode + let mut chunks = vec![]; + + for _ in 0..rng.gen_range(0, 100) { + chunks.push(rng.gen_range(0, MAX_CHUNK)); + } + + FuzzHpack { + seed: seed, + headers: actual, + chunks: chunks, + reduced: 0, + } + } + + fn run(mut self) { + let mut chunks = self.chunks; + let mut headers = self.headers; + let mut expect = headers.clone(); + + let mut encoded = vec![]; + let mut buf = BytesMut::with_capacity( + chunks.pop().unwrap_or(MAX_CHUNK)); + + let mut index = None; + let mut input = headers.into_iter(); + + let mut encoder = Encoder::default(); + let mut decoder = Decoder::default(); + + loop { + match encoder.encode(index.take(), &mut input, &mut buf).unwrap() { + Encode::Full => break, + Encode::Partial(i) => { + index = Some(i); + encoded.push(buf); + buf = BytesMut::with_capacity( + chunks.pop().unwrap_or(MAX_CHUNK)); + } + } + } + + // Decode + for buf in encoded { + decoder.decode(&buf.into(), |e| { + assert_eq!(e, expect.remove(0)); + }); + } + + assert_eq!(0, expect.len()); + } +} + +impl Arbitrary for FuzzHpack { + fn arbitrary(g: &mut G) -> Self { + FuzzHpack::new(quickcheck::Rng::gen(g)) + } + + fn shrink(&self) -> Box> { + let s = self.clone(); + + let iter = (1..self.headers.len()).map(move |i| { + let mut r = s.clone(); + r.headers.drain(i..); + r.reduced = i; + r + }); + + Box::new(iter) + } +} + +fn gen_header(g: &mut StdRng) -> Header { + use http::StatusCode; + use http::method::{self, Method}; + + if g.gen_weighted_bool(10) { + match g.next_u32() % 5 { + 0 => { + let value = gen_string(g, 4, 20); + Header::Authority(value.into()) + } + 1 => { + let method = match g.next_u32() % 6 { + 0 => method::GET, + 1 => method::POST, + 2 => method::PUT, + 3 => method::PATCH, + 4 => method::DELETE, + 5 => { + let n: usize = g.gen_range(3, 7); + let bytes: Vec = (0..n).map(|_| { + g.choose(b"ABCDEFGHIJKLMNOPQRSTUVWXYZ").unwrap().clone() + }).collect(); + + Method::from_bytes(&bytes).unwrap() + } + _ => unreachable!(), + }; + + Header::Method(method) + } + 2 => { + let value = match g.next_u32() % 2 { + 0 => "http", + 1 => "https", + _ => unreachable!(), + }; + + Header::Scheme(value.into()) + } + 3 => { + let value = match g.next_u32() % 100 { + 0 => "/".to_string(), + 1 => "/index.html".to_string(), + _ => gen_string(g, 2, 20), + }; + + Header::Path(value.into()) + } + 4 => { + let status = (g.gen::() % 500) + 100; + + Header::Status(StatusCode::from_u16(status).unwrap()) + } + _ => unreachable!(), + } + } else { + let name = gen_header_name(g); + let mut value = gen_header_value(g); + + if g.gen_weighted_bool(30) { + value.set_sensitive(true); + } + + Header::Field { name: name, value: value } + } +} + +fn gen_header_name(g: &mut StdRng) -> HeaderName { + use http::header; + + if g.gen_weighted_bool(2) { + g.choose(&[ + // TODO: more headers + header::ACCEPT, + ]).unwrap().clone() + } else { + let value = gen_string(g, 1, 25); + HeaderName::from_bytes(value.as_bytes()).unwrap() + } +} + +fn gen_header_value(g: &mut StdRng) -> HeaderValue { + let value = gen_string(g, 0, 70); + HeaderValue::try_from_bytes(value.as_bytes()).unwrap() +} + +fn gen_string(g: &mut StdRng, min: usize, max: usize) -> String { + let bytes: Vec<_> = (min..max).map(|_| { + // Chars to pick from + g.choose(b"ABCDEFGHIJKLMNOPQRSTUVabcdefghilpqrstuvwxyz----").unwrap().clone() + }).collect(); + + String::from_utf8(bytes).unwrap() +} + +/* +impl Arbitrary for HeaderSet { + fn arbitrary(g: &mut G) -> Self { + let mut source: Vec
= vec![]; + + for _ in 0..2000 { + source.push(Header::arbitrary(g)); + } + + // Actual headers + let num: usize = g.gen_range(40, 300); + let mut actual: Vec
= vec![]; + + let skew: i32 = g.gen_range(1, 5); + + while actual.len() < num { + let x: f64 = g.gen_range(0.0, 1.0); + let x = x.powi(skew); + + let i = (x * source.len() as f64) as usize; + actual.push(source[i].clone()); + } + + HeaderSet { + headers: actual, + } + } + + fn shrink(&self) -> Box> { + let headers = self.headers.clone(); + + let iter = (0..headers.len()+1).map(move |i| { + HeaderSet { headers: headers[..0].to_vec() } + }); + + Box::new(iter) + } +} + +impl Arbitrary for Header { + fn arbitrary(g: &mut G) -> Self { + use http::StatusCode; + use http::method::{self, Method}; + + if g.gen_weighted_bool(10) { + match g.next_u32() % 5 { + 0 => { + let value = String::arbitrary(g); + Header::Authority(value.into()) + } + 1 => { + let method = match g.next_u32() % 6 { + 0 => method::GET, + 1 => method::POST, + 2 => method::PUT, + 3 => method::PATCH, + 4 => method::DELETE, + 5 => { + let n = g.gen::() % 7; + let bytes: Vec = (0..n).map(|_| { + g.choose(b"ABCDEFGHIJKLMNOPQRSTUVWXYZ").unwrap().clone() + }).collect(); + + Method::from_bytes(&bytes).unwrap() + } + _ => unreachable!(), + }; + + Header::Method(method) + } + 2 => { + let value = match g.next_u32() % 2 { + 0 => "http", + 1 => "https", + _ => unreachable!(), + }; + + Header::Scheme(value.into()) + } + 3 => { + let value = match g.next_u32() % 100 { + 0 => "/".to_string(), + 1 => "/index.html".to_string(), + _ => String::arbitrary(g), + }; + + Header::Path(value.into()) + } + 4 => { + let status = (g.gen::() % 500) + 100; + + Header::Status(StatusCode::from_u16(status).unwrap()) + } + _ => unreachable!(), + } + } else { + let mut name = HeaderName2::arbitrary(g); + let mut value = HeaderValue2::arbitrary(g); + + if g.gen_weighted_bool(30) { + value.0.set_sensitive(true); + } + + Header::Field { name: name.0, value: value.0 } + } + } +} + +#[derive(Clone)] +struct HeaderName2(HeaderName); + +#[derive(Clone)] +struct HeaderValue2(HeaderValue); + +impl Arbitrary for HeaderName2 { + fn arbitrary(g: &mut G) -> Self { + use http::header; + + if g.gen_weighted_bool(2) { + g.choose(&[ + HeaderName2(header::ACCEPT), + ]).unwrap().clone() + } else { + let len = g.gen::() % 25 + 1; + + let value: Vec = (0..len).map(|_| { + g.choose(b"abcdefghijklmnopqrstuvwxyz-").unwrap().clone() + }).collect(); + + HeaderName2(HeaderName::from_bytes(&value).unwrap()) + } + } +} + +impl Arbitrary for HeaderValue2 { + fn arbitrary(g: &mut G) -> Self { + // Random length + let len = g.gen::() % 70; + + // Generate the value + let value: Vec = (0..len).map(|_| { + g.choose(b"abcdefghijklmnopqrstuvwxyz -_").unwrap().clone() + }).collect(); + + HeaderValue2(HeaderValue::try_from_bytes(&value).unwrap()) + } +} +*/ #[test] fn hpack_fixtures() { @@ -129,7 +523,7 @@ fn test_story(story: Value) { Header::new(name.clone().into(), value.clone().into()).unwrap() }).collect(); - encoder.encode(input.clone(), &mut buf).unwrap(); + encoder.encode(None, &mut input.clone().into_iter(), &mut buf).unwrap(); decoder.decode(&buf.into(), |e| { assert_eq!(e, input.remove(0));