diff --git a/protobuf-fuzz/fuzz/Cargo.toml b/protobuf-fuzz/fuzz/Cargo.toml index 306da37de..5a0e61adf 100644 --- a/protobuf-fuzz/fuzz/Cargo.toml +++ b/protobuf-fuzz/fuzz/Cargo.toml @@ -19,3 +19,7 @@ members = ["."] [[bin]] name = "empty_message" path = "fuzz_targets/empty_message.rs" + +[[bin]] +name = "empty_message_read" +path = "fuzz_targets/empty_message_read.rs" diff --git a/protobuf-fuzz/fuzz/fuzz_targets/empty_message_read.rs b/protobuf-fuzz/fuzz/fuzz_targets/empty_message_read.rs new file mode 100644 index 000000000..9334a4a67 --- /dev/null +++ b/protobuf-fuzz/fuzz/fuzz_targets/empty_message_read.rs @@ -0,0 +1,7 @@ +#![no_main] +#[macro_use] extern crate libfuzzer_sys; +extern crate protobuf_fuzz; + +fuzz_target!(|data: &[u8]| { + protobuf_fuzz::fuzz_target_empty_message_read(data) +}); diff --git a/protobuf-fuzz/src/lib.rs b/protobuf-fuzz/src/lib.rs index f035c620f..01495eac0 100644 --- a/protobuf-fuzz/src/lib.rs +++ b/protobuf-fuzz/src/lib.rs @@ -1,7 +1,14 @@ extern crate protobuf; +use std::io::BufReader; + pub mod all_types_pb; pub fn fuzz_target_empty_message(bytes: &[u8]) { drop(protobuf::parse_from_bytes::(bytes)); } + +pub fn fuzz_target_empty_message_read(bytes: &[u8]) { + let mut reader = BufReader::new(bytes); + drop(protobuf::parse_from_reader::(&mut reader)); +} diff --git a/protobuf/src/buf_read_iter.rs b/protobuf/src/buf_read_iter.rs index 50823c660..4a997718f 100644 --- a/protobuf/src/buf_read_iter.rs +++ b/protobuf/src/buf_read_iter.rs @@ -102,7 +102,7 @@ impl<'ignore> BufReadIter<'ignore> { pos_within_buf: 0, limit_within_buf: bytes.len(), pos_of_buf_start: 0, - limit: bytes.len() as u64, + limit: NO_LIMIT, } } @@ -114,7 +114,7 @@ impl<'ignore> BufReadIter<'ignore> { pos_within_buf: 0, limit_within_buf: bytes.len(), pos_of_buf_start: 0, - limit: bytes.len() as u64, + limit: NO_LIMIT, } } diff --git a/protobuf/src/error.rs b/protobuf/src/error.rs index 32e41a4a5..85af7f9a9 100644 --- a/protobuf/src/error.rs +++ b/protobuf/src/error.rs @@ -19,6 +19,7 @@ pub enum WireError { Utf8Error, InvalidEnumValue(i32), OverRecursionLimit, + TruncatedMessage, Other, } @@ -57,6 +58,7 @@ impl Error for ProtobufError { WireError::IncompleteMap => "incomplete map", WireError::UnexpectedEof => "unexpected EOF", WireError::OverRecursionLimit => "over recursion limit", + WireError::TruncatedMessage => "truncated message", WireError::Other => "other error", } } diff --git a/protobuf/src/stream.rs b/protobuf/src/stream.rs index c151a132a..52ffd32b1 100644 --- a/protobuf/src/stream.rs +++ b/protobuf/src/stream.rs @@ -33,6 +33,9 @@ const OUTPUT_STREAM_BUFFER_SIZE: usize = 8 * 1024; // Default recursion level limit. 100 is the default value of C++'s implementation. const DEFAULT_RECURSION_LIMIT: u32 = 100; +// Max allocated vec when reading length-delimited from unknown input stream +const READ_RAW_BYTES_MAX_ALLOC: usize = 10_000_000; + pub mod wire_format { // TODO: temporary @@ -628,19 +631,30 @@ impl<'a> CodedInputStream<'a> { // TODO: also do some limits when reading from unlimited source if count as u64 > self.source.bytes_until_limit() { - return Err(ProtobufError::WireError(WireError::OverRecursionLimit)); + return Err(ProtobufError::WireError(WireError::TruncatedMessage)); } unsafe { target.set_len(0); } - target.reserve(count); - unsafe { - target.set_len(count); + + if count >= READ_RAW_BYTES_MAX_ALLOC { + // avoid calling `reserve` on buf with very large buffer: could be a malformed message + + let mut take = self.by_ref().take(count as u64); + take.read_to_end(target)?; + + if target.len() != count { + return Err(ProtobufError::WireError(WireError::TruncatedMessage)); + } + } else { + target.reserve(count); + unsafe { + target.set_len(count); + } + + self.source.read_exact(target)?; } - // () is here to make sure correct overload is called - // TODO: rename `read` function - let () = self.read(target)?; Ok(()) } @@ -1266,6 +1280,7 @@ mod test { use super::wire_format; use super::CodedInputStream; use super::CodedOutputStream; + use super::READ_RAW_BYTES_MAX_ALLOC; fn test_read_partial(hex: &str, mut callback: F) where @@ -1436,6 +1451,32 @@ mod test { }); } + #[test] + fn test_input_stream_read_raw_bytes_into_huge() { + let mut v = Vec::new(); + for i in 0..READ_RAW_BYTES_MAX_ALLOC + 1000 { + v.push((i % 10) as u8); + } + + let mut slice: &[u8] = v.as_slice(); + + let mut is = CodedInputStream::new(&mut slice); + + let mut buf = Vec::new(); + + is.read_raw_bytes_into(READ_RAW_BYTES_MAX_ALLOC as u32 + 10, &mut buf).expect("read"); + + assert_eq!(READ_RAW_BYTES_MAX_ALLOC + 10, buf.len()); + + buf.clear(); + + is.read_raw_bytes_into(1000 - 10, &mut buf).expect("read"); + + assert_eq!(1000 - 10, buf.len()); + + assert!(is.eof().expect("eof")); + } + fn test_write(expected: &str, mut gen: F) where F : FnMut(&mut CodedOutputStream) -> ProtobufResult<()>,