diff --git a/CHANGELOG.md b/CHANGELOG.md index d69961a8..4b7971c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +**AIFF**: Stop relying on the file-provided size when reading (Fixes OOM) + ## [0.7.1] - 2022-07-08 ### Added diff --git a/src/id3/v2/write/chunk_file.rs b/src/id3/v2/write/chunk_file.rs index 0e391ab6..1d00bcd8 100644 --- a/src/id3/v2/write/chunk_file.rs +++ b/src/id3/v2/write/chunk_file.rs @@ -16,7 +16,7 @@ where let mut id3v2_chunk = (None, None); - let mut chunks = Chunks::::new(file_size); + let mut chunks = Chunks::::new(u64::from(file_size)); // TODO while chunks.next(data).is_ok() { if &chunks.fourcc == b"ID3 " || &chunks.fourcc == b"id3 " { diff --git a/src/iff/aiff/read.rs b/src/iff/aiff/read.rs index d4956137..675b3ecd 100644 --- a/src/iff/aiff/read.rs +++ b/src/iff/aiff/read.rs @@ -8,13 +8,13 @@ use crate::id3::v2::tag::ID3v2Tag; use crate::iff::chunk::Chunks; use crate::properties::FileProperties; -use std::io::{Read, Seek}; +use std::io::{Read, Seek, SeekFrom}; use byteorder::BigEndian; #[cfg(feature = "aiff_text_chunks")] use byteorder::ReadBytesExt; -pub(in crate::iff) fn verify_aiff(data: &mut R) -> Result +pub(in crate::iff) fn verify_aiff(data: &mut R) -> Result<()> where R: Read + Seek, { @@ -25,16 +25,21 @@ where return Err(LoftyError::new(ErrorKind::UnknownFormat)); } - Ok(u32::from_be_bytes( - id[4..8].try_into().unwrap(), // Infallible - )) + Ok(()) } pub(crate) fn read_from(data: &mut R, read_properties: bool) -> Result where R: Read + Seek, { - let file_size = verify_aiff(data)?; + // TODO: Maybe one day the `Seek` bound can be removed? + // let file_size = verify_aiff(data)?; + verify_aiff(data)?; + + let current_pos = data.stream_position()?; + let file_len = data.seek(SeekFrom::End(0))?; + + data.seek(SeekFrom::Start(current_pos))?; let mut comm = None; let mut stream_len = 0; @@ -49,7 +54,7 @@ where #[cfg(feature = "id3v2")] let mut id3v2_tag: Option = None; - let mut chunks = Chunks::::new(file_size); + let mut chunks = Chunks::::new(file_len); while chunks.next(data).is_ok() { match &chunks.fourcc { diff --git a/src/iff/aiff/tag.rs b/src/iff/aiff/tag.rs index 1f385332..c0b6fd4b 100644 --- a/src/iff/aiff/tag.rs +++ b/src/iff/aiff/tag.rs @@ -357,13 +357,14 @@ where } fn write_to_inner(data: &mut File, mut tag: AiffTextChunksRef<'_, T, AI>) -> Result<()> { - let file_size = super::read::verify_aiff(data)?; + super::read::verify_aiff(data)?; + let file_len = data.metadata()?.len(); let text_chunks = Self::create_text_chunks(&mut tag)?; let mut chunks_remove = Vec::new(); - let mut chunks = Chunks::::new(file_size); + let mut chunks = Chunks::::new(file_len); while chunks.next(data).is_ok() { match &chunks.fourcc { diff --git a/src/iff/chunk.rs b/src/iff/chunk.rs index 3f340c60..3f18499a 100644 --- a/src/iff/chunk.rs +++ b/src/iff/chunk.rs @@ -14,16 +14,16 @@ where { pub fourcc: [u8; 4], pub size: u32, - file_size: u32, + remaining_size: u64, _phantom: PhantomData, } impl Chunks { - pub fn new(file_size: u32) -> Self { + pub fn new(file_size: u64) -> Self { Self { fourcc: [0; 4], size: 0, - file_size, + remaining_size: file_size, _phantom: PhantomData, } } @@ -35,6 +35,8 @@ impl Chunks { data.read_exact(&mut self.fourcc)?; self.size = data.read_u32::()?; + self.remaining_size = self.remaining_size.saturating_sub(8); + Ok(()) } @@ -56,7 +58,7 @@ impl Chunks { R: Read + Seek, { let cont = if let Some(size) = size { - self.read(data, size)? + self.read(data, u64::from(size))? } else { self.content(data)? }; @@ -72,20 +74,21 @@ impl Chunks { where R: Read, { - self.read(data, self.size) + self.read(data, u64::from(self.size)) } - fn read(&self, data: &mut R, size: u32) -> Result> + fn read(&mut self, data: &mut R, size: u64) -> Result> where R: Read, { - if size + 4 > self.file_size { + if size > self.remaining_size { return Err(LoftyError::new(ErrorKind::TooMuchData)); } let mut content = try_vec![0; size as usize]; data.read_exact(&mut content)?; + self.remaining_size = self.remaining_size.saturating_sub(size); Ok(content) } @@ -122,6 +125,8 @@ impl Chunks { data.seek(SeekFrom::Current(i64::from(self.size)))?; self.correct_position(data)?; + self.remaining_size = self.remaining_size.saturating_sub(u64::from(self.size)); + Ok(()) } @@ -134,6 +139,7 @@ impl Chunks { // and it is NOT included in the chunk's size if self.size % 2 != 0 { data.seek(SeekFrom::Current(1))?; + self.remaining_size = self.remaining_size.saturating_sub(1); } Ok(()) diff --git a/src/iff/wav/read.rs b/src/iff/wav/read.rs index 78595e5c..2a04d623 100644 --- a/src/iff/wav/read.rs +++ b/src/iff/wav/read.rs @@ -51,7 +51,7 @@ where #[cfg(feature = "id3v2")] let mut id3v2_tag: Option = None; - let mut chunks = Chunks::::new(file_size); + let mut chunks = Chunks::::new(u64::from(file_size)); while chunks.next(data).is_ok() { match &chunks.fourcc { diff --git a/src/iff/wav/tag/mod.rs b/src/iff/wav/tag/mod.rs index a5f225ab..99ced4b3 100644 --- a/src/iff/wav/tag/mod.rs +++ b/src/iff/wav/tag/mod.rs @@ -277,7 +277,7 @@ mod tests { super::read::parse_riff_info( &mut Cursor::new(&tag[..]), - &mut Chunks::::new(tag.len() as u32), + &mut Chunks::::new(tag.len() as u64), (tag.len() - 1) as u64, &mut parsed_tag, ) @@ -293,7 +293,7 @@ mod tests { super::read::parse_riff_info( &mut Cursor::new(&tag[..]), - &mut Chunks::::new(tag.len() as u32), + &mut Chunks::::new(tag.len() as u64), (tag.len() - 1) as u64, &mut parsed_tag, ) @@ -307,7 +307,7 @@ mod tests { // Remove the LIST....INFO from the tag super::read::parse_riff_info( &mut Cursor::new(&writer[12..]), - &mut Chunks::::new(tag.len() as u32), + &mut Chunks::::new(tag.len() as u64), (tag.len() - 13) as u64, &mut temp_parsed_tag, ) @@ -325,7 +325,7 @@ mod tests { super::read::parse_riff_info( &mut reader, - &mut Chunks::::new(tag_bytes.len() as u32), + &mut Chunks::::new(tag_bytes.len() as u64), (tag_bytes.len() - 1) as u64, &mut riff_info, ) diff --git a/src/iff/wav/tag/write.rs b/src/iff/wav/tag/write.rs index e1d0a54a..e332a9fd 100644 --- a/src/iff/wav/tag/write.rs +++ b/src/iff/wav/tag/write.rs @@ -57,7 +57,7 @@ where { let mut info = None; - let mut chunks = Chunks::::new(file_size); + let mut chunks = Chunks::::new(u64::from(file_size)); while chunks.next(data).is_ok() { if &chunks.fourcc == b"LIST" { diff --git a/tests/fuzz/aifffile_read_from.rs b/tests/fuzz/aifffile_read_from.rs index e69de29b..21896459 100644 --- a/tests/fuzz/aifffile_read_from.rs +++ b/tests/fuzz/aifffile_read_from.rs @@ -0,0 +1,7 @@ +use crate::oom_test; +use lofty::iff::AiffFile; + +#[test] +fn oom1() { + oom_test::("aifffile_read_from/oom-88065007d35ee271d5812fd723a3b458488813ea"); +} diff --git a/tests/fuzz/flacfile_read_from.rs b/tests/fuzz/flacfile_read_from.rs index e69de29b..8b137891 100644 --- a/tests/fuzz/flacfile_read_from.rs +++ b/tests/fuzz/flacfile_read_from.rs @@ -0,0 +1 @@ + diff --git a/tests/fuzz/main.rs b/tests/fuzz/main.rs new file mode 100644 index 00000000..3ae3e5d7 --- /dev/null +++ b/tests/fuzz/main.rs @@ -0,0 +1,37 @@ +use lofty::AudioFile; +use std::io::Cursor; +use std::path::Path; +use std::thread; +use std::time::Instant; + +mod aifffile_read_from; +mod flacfile_read_from; +mod mp3file_read_from; +mod mp4file_read_from; +mod opusfile_read_from; +mod pictureinformation_from_jpeg; +mod pictureinformation_from_png; +mod speexfile_read_from; +mod vorbisfile_read_from; +mod wavfile_read_from; +mod wavpackfile_read_from; + +pub fn get_reader(path: &str) -> Cursor> { + let path = Path::new("tests/fuzz/assets").join(path); + + let b = std::fs::read(path).unwrap(); + Cursor::new(b) +} + +pub fn oom_test(path: &'static str) { + let instant = Instant::now(); + let thread = thread::spawn(|| { + ::read_from(&mut get_reader(path), true).unwrap(); + }); + + while instant.elapsed().as_secs() < 2 {} + + if !thread.is_finished() { + panic!("Failed to run test"); + } +} diff --git a/tests/fuzz/mp3file_read_from.rs b/tests/fuzz/mp3file_read_from.rs index e69de29b..8b137891 100644 --- a/tests/fuzz/mp3file_read_from.rs +++ b/tests/fuzz/mp3file_read_from.rs @@ -0,0 +1 @@ + diff --git a/tests/fuzz/mp4file_read_from.rs b/tests/fuzz/mp4file_read_from.rs index e69de29b..8b137891 100644 --- a/tests/fuzz/mp4file_read_from.rs +++ b/tests/fuzz/mp4file_read_from.rs @@ -0,0 +1 @@ + diff --git a/tests/fuzz/opusfile_read_from.rs b/tests/fuzz/opusfile_read_from.rs index e69de29b..8b137891 100644 --- a/tests/fuzz/opusfile_read_from.rs +++ b/tests/fuzz/opusfile_read_from.rs @@ -0,0 +1 @@ + diff --git a/tests/fuzz/pictureinformation_from_jpeg.rs b/tests/fuzz/pictureinformation_from_jpeg.rs index e69de29b..8b137891 100644 --- a/tests/fuzz/pictureinformation_from_jpeg.rs +++ b/tests/fuzz/pictureinformation_from_jpeg.rs @@ -0,0 +1 @@ + diff --git a/tests/fuzz/pictureinformation_from_png.rs b/tests/fuzz/pictureinformation_from_png.rs index e69de29b..8b137891 100644 --- a/tests/fuzz/pictureinformation_from_png.rs +++ b/tests/fuzz/pictureinformation_from_png.rs @@ -0,0 +1 @@ + diff --git a/tests/fuzz/speexfile_read_from.rs b/tests/fuzz/speexfile_read_from.rs index e69de29b..8b137891 100644 --- a/tests/fuzz/speexfile_read_from.rs +++ b/tests/fuzz/speexfile_read_from.rs @@ -0,0 +1 @@ + diff --git a/tests/fuzz/vorbisfile_read_from.rs b/tests/fuzz/vorbisfile_read_from.rs index e69de29b..8b137891 100644 --- a/tests/fuzz/vorbisfile_read_from.rs +++ b/tests/fuzz/vorbisfile_read_from.rs @@ -0,0 +1 @@ + diff --git a/tests/fuzz/wavfile_read_from.rs b/tests/fuzz/wavfile_read_from.rs index e69de29b..8b137891 100644 --- a/tests/fuzz/wavfile_read_from.rs +++ b/tests/fuzz/wavfile_read_from.rs @@ -0,0 +1 @@ + diff --git a/tests/fuzz/wavpackfile_read_from.rs b/tests/fuzz/wavpackfile_read_from.rs index e69de29b..8b137891 100644 --- a/tests/fuzz/wavpackfile_read_from.rs +++ b/tests/fuzz/wavpackfile_read_from.rs @@ -0,0 +1 @@ +