Skip to content

Commit

Permalink
FIX: properly returned Errors instead of panicking in read(), address…
Browse files Browse the repository at this point in the history
…ing issue #3 reported by Corey Farwell
  • Loading branch information
ende76 committed Oct 25, 2015
1 parent 976331f commit 4882090
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 10 deletions.
22 changes: 18 additions & 4 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2275,10 +2275,24 @@ impl<R: Read> Read for Decompressor<R> {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
if self.buf.is_empty() {
match self.decompress() {
Err(e) => {

panic!(format!("{:?}", e.description()));
},
Err(e @ DecompressorError::UnexpectedEOF) |
Err(e @ DecompressorError::NonZeroFillBit) |
Err(e @ DecompressorError::NonZeroReservedBit) |
Err(e @ DecompressorError::NonZeroTrailerBit) |
Err(e @ DecompressorError::NonZeroTrailerNibble) |
Err(e @ DecompressorError::ExpectedEndOfStream) |
Err(e @ DecompressorError::InvalidMSkipLen) |
Err(e @ DecompressorError::ParseErrorInsertAndCopyLength) |
Err(e @ DecompressorError::ParseErrorInsertLiterals) |
Err(e @ DecompressorError::ParseErrorContextMap) |
Err(e @ DecompressorError::ParseErrorDistanceCode) |
Err(e @ DecompressorError::ExceededExpectedBytes) |
Err(e @ DecompressorError::ParseErrorComplexPrefixCodeLengths) |
Err(e @ DecompressorError::RingBufferError) |
Err(e @ DecompressorError::RunLengthExceededSizeOfContextMap) |
Err(e @ DecompressorError::InvalidTransformId) |
Err(e @ DecompressorError::InvalidLengthInStaticDictionary) |
Err(e @ DecompressorError::CodeLengthsChecksum) => return Err(io::Error::new(io::ErrorKind::InvalidData, e.description())),
Ok(_) => {},
}
}
Expand Down
8 changes: 5 additions & 3 deletions src/main.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
extern crate brotli;

fn main() {
use std::io::{ Cursor, Read };
use std::io::Read;
use brotli::Decompressor;

let mut input = vec![];
let _ = Decompressor::new(Cursor::new(vec![0x1b, 0x3f, 0xff, 0xff, 0xdb, 0x4f, 0xe2, 0x99, 0x80, 0x12])).read_to_end(&mut input);
let mut input = vec![];
let result = Decompressor::new(&b"\xb1".to_vec() as &[u8]).read_to_end(&mut input);

println!("result = {:?}", result);
}

28 changes: 25 additions & 3 deletions tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,17 +300,39 @@ fn should_decompress_alice29_txt() {

#[test]
#[should_panic(expected = "Error parsing code lengths")]
/// frewsxcv: fuzzer-test
/// frewsxcv_00: fuzzer-test
/// exposes endless-loop vulnerability, if runlength code lengths are not bounded by alphabet size
/// found and reported by Corey Farwell – https://github.com/ende76/brotli-rs/issues/2
fn should_reject_frewsxcv() {
fn should_reject_frewsxcv_00() {
use std::io::{ Cursor, Read };
use brotli::Decompressor;

let mut input = vec![];
let _ = Decompressor::new(Cursor::new(vec![0x1b, 0x3f, 0xff, 0xff, 0xdb, 0x4f, 0xe2, 0x99, 0x80, 0x12])).read_to_end(&mut input);
let result = Decompressor::new(Cursor::new(vec![0x1b, 0x3f, 0xff, 0xff, 0xdb, 0x4f, 0xe2, 0x99, 0x80, 0x12])).read_to_end(&mut input);

match result {
Err(e) => panic!("{:?}", e),
_ => {},
}
}

#[test]
#[should_panic(expected = "unexpected EOF")]
/// frewsxcv: fuzzer-test
/// exposes uncaught panic in read() implementation
/// found and reported by Corey Farwell – https://github.com/ende76/brotli-rs/issues/2
fn should_reject_frewsxcv_01() {
use std::io::Read;
use brotli::Decompressor;

let mut input = vec![];
let result = Decompressor::new(&b"\xb1".to_vec() as &[u8]).read_to_end(&mut input);

match result {
Err(e) => panic!("{:?}", e),
_ => {},
}
}

fn inverse_move_to_front_transform(v: &mut[u8]) {
let mut mtf: Vec<u8> = vec![0; 256];
Expand Down

3 comments on commit 4882090

@frewsxcv
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are failing now. Might be good to use pull requests to prevent test-failing code into master?

@frewsxcv
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ende76
Copy link
Owner Author

@ende76 ende76 commented on 4882090 Oct 25, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@frewsxcv Yes, seems like it's time for me to call it a day. ;) I have fixed the tests, and will look into the suggested workflow with pull requests for the future. It does sound like the right way to go.

Please sign in to comment.