Fuzzing fixes

CHANGELOG.md

@@ -39,6 +39,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - When skipping invalid frames in `ParsingMode::{BestAttempt, Relaxed}`, the parser will no longer be able to go out of the bounds
     of the frame content ([issue](https://github.com/Serial-ATA/lofty-rs/issues/458)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/459))
 - **MP4**: Support for flag items (ex. `cpil`) of any size (not just 1 byte) ([issue](https://github.com/Serial-ATA/lofty-rs/issues/457)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/460))
+- **Fuzzing** (Thanks [@qarmin](https://github.com/qarmin)!) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/476)):
+    - **MusePack**: Fix panic when ID3v2 tag sizes exceed the stream length ([issue](https://github.com/Serial-ATA/lofty-rs/issues/470))
+    - **WAV**: Fix panic when calculating bit depth with abnormally large `bytes_per_sample` ([issue](https://github.com/Serial-ATA/lofty-rs/issues/471))
+    - **WavPack***: Fix panic when encountering wrongly sized blocks ([issue](https://github.com/Serial-ATA/lofty-rs/issues/472))
+    - **WavPack***: Fix panic when encountering zero-sized blocks ([issue](https://github.com/Serial-ATA/lofty-rs/issues/473))
+    - **MPEG**: Fix panic when APE tags are incorrectly sized ([issue](https://github.com/Serial-ATA/lofty-rs/issues/474))
 
 ## [0.21.1] - 2024-08-28
 

lofty/src/ape/read.rs

@@ -8,7 +8,7 @@ use crate::id3::v1::tag::Id3v1Tag;
 use crate::id3::v2::read::parse_id3v2;
 use crate::id3::v2::tag::Id3v2Tag;
 use crate::id3::{find_id3v1, find_id3v2, find_lyrics3v2, FindId3v2Config, ID3FindResults};
-use crate::macros::decode_err;
+use crate::macros::{decode_err, err};
 
 use std::io::{Read, Seek, SeekFrom};
 
@@ -37,12 +37,12 @@ where
 	if let ID3FindResults(Some(header), content) = find_id3v2(data, find_id3v2_config)? {
 		log::warn!("Encountered an ID3v2 tag. This tag cannot be rewritten to the APE file!");
 
-		stream_len -= u64::from(header.size);
+		let Some(new_stream_length) = stream_len.checked_sub(u64::from(header.full_tag_size()))
+		else {
+			err!(SizeMismatch);
+		};
 
-		// Exclude the footer
-		if header.flags.footer {
-			stream_len -= 10;
-		}
+		stream_len = new_stream_length;
 
 		if let Some(content) = content {
 			let reader = &mut &*content;
@@ -107,16 +107,21 @@ where
 	let ID3FindResults(id3v1_header, id3v1) = find_id3v1(data, parse_options.read_tags)?;
 
 	if id3v1_header.is_some() {
-		stream_len -= 128;
 		id3v1_tag = id3v1;
+		let Some(new_stream_length) = stream_len.checked_sub(128) else {
+			err!(SizeMismatch);
+		};
+
+		stream_len = new_stream_length;
 	}
 
 	// Next, check for a Lyrics3v2 tag, and skip over it, as it's no use to us
-	let ID3FindResults(lyrics3_header, lyrics3v2_size) = find_lyrics3v2(data)?;
+	let ID3FindResults(_, lyrics3v2_size) = find_lyrics3v2(data)?;
+	let Some(new_stream_length) = stream_len.checked_sub(u64::from(lyrics3v2_size)) else {
+		err!(SizeMismatch);
+	};
 
-	if lyrics3_header.is_some() {
-		stream_len -= u64::from(lyrics3v2_size)
-	}
+	stream_len = new_stream_length;
 
 	// Next, search for an APE tag footer
 	//

lofty/src/iff/wav/properties.rs

@@ -209,7 +209,7 @@ pub(super) fn read_properties(
 			..
 		}) if valid_bits_per_sample > 0 => bit_depth = valid_bits_per_sample as u8,
 		_ if bits_per_sample > 0 => bit_depth = bits_per_sample as u8,
-		_ => bit_depth = (bytes_per_sample * 8) as u8,
+		_ => bit_depth = bytes_per_sample.saturating_mul(8) as u8,
 	};
 
 	let channel_mask = extensible_info.map(|info| info.channel_mask);

lofty/src/mpeg/read.rs

@@ -163,7 +163,11 @@ where
 
 			// Seek back to the start of the tag
 			let pos = reader.stream_position()?;
-			reader.seek(SeekFrom::Start(pos - u64::from(header.size)))?;
+			let Some(start_of_tag) = pos.checked_sub(u64::from(header.size)) else {
+				err!(SizeMismatch);
+			};
+
+			reader.seek(SeekFrom::Start(start_of_tag))?;
 		},
 		_ => {
 			// Correct the position (APE header - Preamble)

lofty/src/musepack/read.rs

@@ -32,12 +32,17 @@ where
 	// ID3v2 tags are unsupported in MPC files, but still possible
 	#[allow(unused_variables)]
 	if let ID3FindResults(Some(header), Some(content)) = find_id3v2(reader, find_id3v2_config)? {
+		let Some(new_stream_length) = stream_length.checked_sub(u64::from(header.full_tag_size()))
+		else {
+			err!(SizeMismatch);
+		};
+
+		stream_length = new_stream_length;
+
 		let reader = &mut &*content;
 
 		let id3v2 = parse_id3v2(reader, header, parse_options)?;
 		file.id3v2_tag = Some(id3v2);
-
-		stream_length -= u64::from(header.full_tag_size());
 	}
 
 	// Save the current position, so we can go back and read the properties after the tags

lofty/src/wavpack/properties.rs

@@ -305,6 +305,10 @@ fn get_extended_meta_info(
 	let mut index = 0;
 	let block_size = block_content.len();
 	while index < block_size {
+		if block_size - index < 2 {
+			break;
+		}
+
 		let id = block_content[index];
 		index += 1;
 
@@ -318,6 +322,10 @@ fn get_extended_meta_info(
 			index += 2;
 		}
 
+		if size == 0 {
+			decode_err!(@BAIL WavPack, "Encountered a zero-sized block");
+		}
+
 		if id & ID_FLAG_ODD_SIZE > 0 {
 			size -= 1;
 		}
@@ -326,6 +334,7 @@ fn get_extended_meta_info(
 			ID_NON_STANDARD_SAMPLE_RATE => {
 				properties.sample_rate =
 					(&mut &block_content[index..]).read_u24::<LittleEndian>()?;
+				index += 3;
 			},
 			ID_DSD => {
 				if size <= 1 {
@@ -364,32 +373,46 @@ fn get_extended_meta_info(
 				match s {
 					0 => {
 						let channel_mask = reader.read_u8()?;
+						index += 1;
+
 						properties.channel_mask = ChannelMask(u32::from(channel_mask));
 					},
 					1 => {
 						let channel_mask = reader.read_u16::<LittleEndian>()?;
+						index += 2;
+
 						properties.channel_mask = ChannelMask(u32::from(channel_mask));
 					},
 					2 => {
 						let channel_mask = reader.read_u24::<LittleEndian>()?;
+						index += 3;
+
 						properties.channel_mask = ChannelMask(channel_mask);
 					},
 					3 => {
 						let channel_mask = reader.read_u32::<LittleEndian>()?;
+						index += 4;
+
 						properties.channel_mask = ChannelMask(channel_mask);
 					},
 					4 => {
 						properties.channels |= u16::from(reader.read_u8()? & 0xF) << 8;
 						properties.channels += 1;
+						index += 1;
 
 						let channel_mask = reader.read_u24::<LittleEndian>()?;
+						index += 3;
+
 						properties.channel_mask = ChannelMask(channel_mask);
 					},
 					5 => {
 						properties.channels |= u16::from(reader.read_u8()? & 0xF) << 8;
 						properties.channels += 1;
+						index += 1;
 
 						let channel_mask = reader.read_u32::<LittleEndian>()?;
+						index += 4;
+
 						properties.channel_mask = ChannelMask(channel_mask);
 					},
 					_ => decode_err!(@BAIL WavPack, "Encountered invalid channel info size"),

lofty/src/wavpack/read.rs

@@ -4,6 +4,7 @@ use crate::config::ParseOptions;
 use crate::error::Result;
 use crate::id3::{find_id3v1, find_lyrics3v2, ID3FindResults};
 
+use crate::macros::err;
 use std::io::{Read, Seek, SeekFrom};
 
 pub(super) fn read_from<R>(reader: &mut R, parse_options: ParseOptions) -> Result<WavPackFile>
@@ -20,16 +21,21 @@ where
 	let ID3FindResults(id3v1_header, id3v1) = find_id3v1(reader, parse_options.read_tags)?;
 
 	if id3v1_header.is_some() {
-		stream_length -= 128;
 		id3v1_tag = id3v1;
+		let Some(new_stream_length) = stream_length.checked_sub(128) else {
+			err!(SizeMismatch);
+		};
+
+		stream_length = new_stream_length;
 	}
 
 	// Next, check for a Lyrics3v2 tag, and skip over it, as it's no use to us
-	let ID3FindResults(lyrics3_header, lyrics3v2_size) = find_lyrics3v2(reader)?;
+	let ID3FindResults(_, lyrics3v2_size) = find_lyrics3v2(reader)?;
+	let Some(new_stream_length) = stream_length.checked_sub(u64::from(lyrics3v2_size)) else {
+		err!(SizeMismatch);
+	};
 
-	if lyrics3_header.is_some() {
-		stream_length -= u64::from(lyrics3v2_size);
-	}
+	stream_length = new_stream_length;
 
 	// Next, search for an APE tag footer
 	//

lofty/tests/fuzz/mpcfile_read_from.rs

@@ -8,3 +8,11 @@ fn panic1() {
 	let mut reader = crate::get_reader("mpcfile_read_from/output.aac");
 	let _ = MpcFile::read_from(&mut reader, ParseOptions::new());
 }
+
+// Overflow on badly sized ID3v2 tag
+#[test_log::test]
+fn panic2() {
+	let mut reader =
+		crate::get_reader("mpcfile_read_from/crash-c98d99ebce28b50b50eb2e96320f02e1e729e543");
+	let _ = MpcFile::read_from(&mut reader, ParseOptions::new());
+}

lofty/tests/fuzz/mpegfile_read_from.rs

@@ -19,6 +19,13 @@ fn crash2() {
 	let _ = MpegFile::read_from(&mut reader, ParseOptions::new());
 }
 
+#[test_log::test]
+fn crash3() {
+	let mut reader =
+		get_reader("mpegfile_read_from/crash-7b5eb183cc14faf3ecc93bdf4a5e30b05f7a537d_minimized");
+	let _ = MpegFile::read_from(&mut reader, ParseOptions::new());
+}
+
 #[test_log::test]
 fn oom1() {
 	oom_test::<MpegFile>("mpegfile_read_from/oom-f8730cbfa5682ab12343ccb70de9b71a061ef4d0");

lofty/tests/fuzz/wavfile_read_from.rs

@@ -28,3 +28,9 @@ fn panic3() {
 		crate::get_reader("wavfile_read_from/2_IDX_34_RAND_128635499166458268533001.wav");
 	let _ = WavFile::read_from(&mut reader, ParseOptions::new());
 }
+
+#[test_log::test]
+fn panic4() {
+	let mut reader = crate::get_reader("wavfile_read_from/aa");
+	let _ = WavFile::read_from(&mut reader, ParseOptions::new());
+}

lofty/tests/fuzz/wavpackfile_read_from.rs

@@ -1,4 +1,6 @@
 use crate::oom_test;
+use lofty::config::ParseOptions;
+use lofty::file::AudioFile;
 use lofty::wavpack::WavPackFile;
 
 #[test_log::test]
@@ -82,3 +84,15 @@ fn oom11() {
 fn oom12() {
 	oom_test::<WavPackFile>("wavpackfile_read_from/oom-94867b6fefcd32cd5bc3bc298468cd3d65d93ff1");
 }
+
+#[test_log::test]
+fn panic1() {
+	let mut reader = crate::get_reader("wavpackfile_read_from/output");
+	let _ = WavPackFile::read_from(&mut reader, ParseOptions::default());
+}
+
+#[test_log::test]
+fn panic2() {
+	let mut reader = crate::get_reader("wavpackfile_read_from/bb");
+	let _ = WavPackFile::read_from(&mut reader, ParseOptions::default());
+}