AIFF: Stop relying on file's provided size (Fix OOM)

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

src/id3/v2/write/chunk_file.rs

@@ -16,7 +16,7 @@ where
 
 	let mut id3v2_chunk = (None, None);
 
-	let mut chunks = Chunks::<B>::new(file_size);
+	let mut chunks = Chunks::<B>::new(u64::from(file_size)); // TODO
 
 	while chunks.next(data).is_ok() {
 		if &chunks.fourcc == b"ID3 " || &chunks.fourcc == b"id3 " {

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<R>(data: &mut R) -> Result<u32>
+pub(in crate::iff) fn verify_aiff<R>(data: &mut R) -> Result<()>
 where
 	R: Read + Seek,
 {
@@ -25,16 +25,21 @@ where
 		return Err(LoftyError::new(ErrorKind::UnknownFormat));
 	}
 
-	Ok(u32::from_be_bytes(
+	Ok(())
-		id[4..8].try_into().unwrap(), // Infallible
-	))
 }
 
 pub(crate) fn read_from<R>(data: &mut R, read_properties: bool) -> Result<AiffFile>
 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<ID3v2Tag> = None;
 
-	let mut chunks = Chunks::<BigEndian>::new(file_size);
+	let mut chunks = Chunks::<BigEndian>::new(file_len);
 
 	while chunks.next(data).is_ok() {
 		match &chunks.fourcc {

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::<BigEndian>::new(file_size);
+		let mut chunks = Chunks::<BigEndian>::new(file_len);
 
 		while chunks.next(data).is_ok() {
 			match &chunks.fourcc {

src/iff/chunk.rs

@@ -14,16 +14,16 @@ where
 {
 	pub fourcc: [u8; 4],
 	pub size: u32,
-	file_size: u32,
+	remaining_size: u64,
 	_phantom: PhantomData<B>,
 }
 
 impl<B: ByteOrder> Chunks<B> {
-	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<B: ByteOrder> Chunks<B> {
 		data.read_exact(&mut self.fourcc)?;
 		self.size = data.read_u32::<B>()?;
 
+		self.remaining_size = self.remaining_size.saturating_sub(8);
+
 		Ok(())
 	}
 
@@ -56,7 +58,7 @@ impl<B: ByteOrder> Chunks<B> {
 		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<B: ByteOrder> Chunks<B> {
 	where
 		R: Read,
 	{
-		self.read(data, self.size)
+		self.read(data, u64::from(self.size))
 	}
 
-	fn read<R>(&self, data: &mut R, size: u32) -> Result<Vec<u8>>
+	fn read<R>(&mut self, data: &mut R, size: u64) -> Result<Vec<u8>>
 	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<B: ByteOrder> Chunks<B> {
 		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<B: ByteOrder> Chunks<B> {
 		// 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(())

src/iff/wav/read.rs

@@ -51,7 +51,7 @@ where
 	#[cfg(feature = "id3v2")]
 	let mut id3v2_tag: Option<ID3v2Tag> = None;
 
-	let mut chunks = Chunks::<LittleEndian>::new(file_size);
+	let mut chunks = Chunks::<LittleEndian>::new(u64::from(file_size));
 
 	while chunks.next(data).is_ok() {
 		match &chunks.fourcc {

src/iff/wav/tag/mod.rs

@@ -277,7 +277,7 @@ mod tests {
 
 		super::read::parse_riff_info(
 			&mut Cursor::new(&tag[..]),
-			&mut Chunks::<LittleEndian>::new(tag.len() as u32),
+			&mut Chunks::<LittleEndian>::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::<LittleEndian>::new(tag.len() as u32),
+			&mut Chunks::<LittleEndian>::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::<LittleEndian>::new(tag.len() as u32),
+			&mut Chunks::<LittleEndian>::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::<LittleEndian>::new(tag_bytes.len() as u32),
+			&mut Chunks::<LittleEndian>::new(tag_bytes.len() as u64),
 			(tag_bytes.len() - 1) as u64,
 			&mut riff_info,
 		)

src/iff/wav/tag/write.rs

@@ -57,7 +57,7 @@ where
 {
 	let mut info = None;
 
-	let mut chunks = Chunks::<LittleEndian>::new(file_size);
+	let mut chunks = Chunks::<LittleEndian>::new(u64::from(file_size));
 
 	while chunks.next(data).is_ok() {
 		if &chunks.fourcc == b"LIST" {

tests/fuzz/aifffile_read_from.rs

@@ -0,0 +1,7 @@
+use crate::oom_test;
+use lofty::iff::AiffFile;
+
+#[test]
+fn oom1() {
+	oom_test::<AiffFile>("aifffile_read_from/oom-88065007d35ee271d5812fd723a3b458488813ea");
+}

tests/fuzz/flacfile_read_from.rs

@@ -0,0 +1 @@
+

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<Vec<u8>> {
+	let path = Path::new("tests/fuzz/assets").join(path);
+
+	let b = std::fs::read(path).unwrap();
+	Cursor::new(b)
+}
+
+pub fn oom_test<A: AudioFile>(path: &'static str) {
+	let instant = Instant::now();
+	let thread = thread::spawn(|| {
+		<A as AudioFile>::read_from(&mut get_reader(path), true).unwrap();
+	});
+
+	while instant.elapsed().as_secs() < 2 {}
+
+	if !thread.is_finished() {
+		panic!("Failed to run test");
+	}
+}

tests/fuzz/mp3file_read_from.rs

@@ -0,0 +1 @@
+

tests/fuzz/mp4file_read_from.rs

@@ -0,0 +1 @@
+

tests/fuzz/opusfile_read_from.rs

@@ -0,0 +1 @@
+

tests/fuzz/pictureinformation_from_jpeg.rs

@@ -0,0 +1 @@
+

tests/fuzz/pictureinformation_from_png.rs

@@ -0,0 +1 @@
+

tests/fuzz/speexfile_read_from.rs

@@ -0,0 +1 @@
+

tests/fuzz/vorbisfile_read_from.rs

@@ -0,0 +1 @@
+

tests/fuzz/wavfile_read_from.rs

@@ -0,0 +1 @@
+

tests/fuzz/wavpackfile_read_from.rs

@@ -0,0 +1 @@
+