Skip to content

Commit

Permalink
Fix crash in #32
Browse files Browse the repository at this point in the history
Badly formatted files managed to allocate giant buffers in release mode,
and trigger an subtraction overflow in debug mode.

While a large allocation doesn't imply a bug, the overflow certainly is
one.
  • Loading branch information
est31 committed Sep 8, 2018
1 parent 462f868 commit b20e267
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 17 deletions.
7 changes: 6 additions & 1 deletion dev/cmp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,12 +522,17 @@ pub fn get_xiph_asset_defs_5() -> [TestAssetDef; 5] {
}

#[allow(unused)]
pub fn get_malformed_asset_defs() -> [TestAssetDef; 1] {
pub fn get_malformed_asset_defs() -> [TestAssetDef; 2] {
return [
TestAssetDef {
filename : format!("27_really_minimized_testcase_crcfix.ogg"),
hash : format!("83f6d6f36ae926000f064007e79ef7c45ed561e49223d9b68f980d264050d683"),
url : format!("https://github.com/RustAudio/lewton/files/2363013/27_really_minimized_testcase_crcfix.ogg.zip"),
},
TestAssetDef {
filename : format!("32_minimized_crash_testcase.ogg"),
hash : format!("644170ccc3e48f2e2bf28cadddcd837520df09671c3d3d991b128b9fdb281da6"),
url : format!("https://github.com/RustAudio/lewton/files/2363080/32_minimized_crash_testcase.ogg.zip"),
},
];
}
40 changes: 24 additions & 16 deletions dev/cmp/tests/malformed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ extern crate lewton;

use std::fs::File;
use lewton::inside_ogg::OggStreamReader;
use lewton::VorbisError;

macro_rules! try {
($expr:expr) => (match $expr {
Expand All @@ -23,25 +22,31 @@ macro_rules! try {
})
}

macro_rules! etry {
($expr:expr, $expected:pat, $action:tt) => (match $expr {
Ok(val) => val,
Err($expected) => {
$action
},
Err(e) => {
panic!("Unexpected error: {:?}\nExpected: {:?}", e, stringify!($type));
},
})
}

// Ensures that a file is malformed and returns an error,
// but
macro_rules! ensure_malformed {
($name:expr, $type:expr) => {{
($name:expr, $expected:pat) => {{
// Read the file to memory
let f = try!(File::open(format!("test-assets/{}", $name)));
let mut ogg_rdr = try!(OggStreamReader::new(f));
loop {
match ogg_rdr.read_dec_packet_itl() {
Ok(Some(_)) => (),
Ok(None) => panic!("File {} decoded without errors", $name),
Err(VorbisError::BadAudio(e)) => {
assert_eq!(e, $type);
break;
},
Err(e) => {
panic!("Unexpected error: {:?}", e);
},
};
if let Some(mut ogg_rdr) = etry!(OggStreamReader::new(f).map(|v| Some(v)), $expected, None) {
loop {
match etry!(ogg_rdr.read_dec_packet_itl(), $expected, break) {
Some(_) => (),
None => panic!("File {} decoded without errors", $name),
};
}
}
}}
}
Expand All @@ -53,7 +58,10 @@ fn test_malformed_fuzzed() {
"test-assets", true).unwrap();
println!();

use lewton::VorbisError::*;
use lewton::audio::AudioReadError::*;
use lewton::header::HeaderReadError::*;

ensure_malformed!("27_really_minimized_testcase_crcfix.ogg", AudioBadFormat);
ensure_malformed!("27_really_minimized_testcase_crcfix.ogg", BadAudio(AudioBadFormat));
ensure_malformed!("32_minimized_crash_testcase.ogg", BadHeader(HeaderBadFormat));
}
8 changes: 8 additions & 0 deletions src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -918,6 +918,14 @@ fn read_residue(rdr :&mut BitpackCursor, codebooks :&Vec<Codebook>)
}
let residue_begin = try!(rdr.read_u24());
let residue_end = try!(rdr.read_u24());
if residue_begin > residue_end {
// If residue_begin < residue_end, we'll get
// errors in audio parsing code.
// As the idea of residue end being before begin
// sounds quite wrong anyway, we already error
// earlier, in header parsing code.
try!(Err(HeaderReadError::HeaderBadFormat));
}
let residue_partition_size = try!(rdr.read_u24()) + 1;
let residue_classifications = try!(rdr.read_u6()) + 1;
let residue_classbook = try!(rdr.read_u8());
Expand Down

0 comments on commit b20e267

Please sign in to comment.