Fix more panics from fuzzing

CHANGELOG.md

@@ -25,13 +25,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **MP4**: Atoms with sizes greater than the remaining file size will be ignored with `ParsingMode::Relaxed` ([PR](https://github.com/Serial-ATA/lofty-rs/pull/433))
 
 ### Fixed
-- **Fuzzing** (Thanks [@qarmin](https://github.com/qarmin)!) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/TODO)):
+- **Fuzzing** (Thanks [@qarmin](https://github.com/qarmin)!) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/423)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/434)):
   - **MP4**:
     - Fix panic when reading properties of a file with no timescale specified ([issue](https://github.com/Serial-ATA/lofty-rs/issues/418))
     - Fix panics when reading improperly sized freeform atom identifiers ([issue](https://github.com/Serial-ATA/lofty-rs/issues/425)) ([issue](https://github.com/Serial-ATA/lofty-rs/issues/426))
+    - Fix panic when `data` atom length is less than 16 bytes ([issue](https://github.com/Serial-ATA/lofty-rs/issues/429))
+    - Fix panic with improperly sized freeform identifiers ([issue](https://github.com/Serial-ATA/lofty-rs/issues/430))
+    - Fix panic when `hdlr` atom is an unexpected length ([issue](https://github.com/Serial-ATA/lofty-rs/issues/435))
   - **WAV**:
     - Fix panic when reading properties with large written bytes per second ([issue](https://github.com/Serial-ATA/lofty-rs/issues/420))
     - Fix panic when reading an improperly sized INFO LIST ([issue](https://github.com/Serial-ATA/lofty-rs/issues/427))
+    - Fix panic when reading a fmt chunk with an invalid bits_per_sample field ([issue](https://github.com/Serial-ATA/lofty-rs/issues/428))
   - **Vorbis**:
     - Fix panic when reading properties of a file with large absolute granule positions ([issue](https://github.com/Serial-ATA/lofty-rs/issues/421))
     - Fix attempted large allocations with invalid comment counts ([issue](https://github.com/Serial-ATA/lofty-rs/issues/419))

lofty/src/iff/wav/properties.rs

@@ -196,7 +196,12 @@ pub(super) fn read_properties(
 		decode_err!(@BAIL Wav, "File contains 0 channels");
 	}
 
+	if bits_per_sample % 8 != 0 {
+		decode_err!(@BAIL Wav, "Bits per sample is not a multiple of 8");
+	}
+
 	let bytes_per_sample = block_align / u16::from(channels);
+
 	let bit_depth;
 	match extensible_info {
 		Some(ExtensibleFmtChunk {
@@ -215,7 +220,7 @@ pub(super) fn read_properties(
 	}
 
 	if bits_per_sample > 0 && (total_samples == 0 || pcm) {
-		total_samples = stream_len / u32::from(u16::from(channels) * ((bits_per_sample + 7) / 8))
+		total_samples = stream_len / (u32::from(channels) * u32::from(bits_per_sample / 8));
 	}
 
 	let mut duration = Duration::ZERO;

lofty/src/mp4/atom_info.rs

@@ -199,7 +199,7 @@ impl AtomInfo {
 				err!(BadAtom("Found an incomplete freeform identifier"));
 			}
 
-			atom_ident = parse_freeform(data, len, reader_size, parse_mode)?;
+			atom_ident = parse_freeform(data, len - ATOM_HEADER_LEN, parse_mode)?;
 		} else {
 			atom_ident = AtomIdent::Fourcc(identifier);
 		}
@@ -224,7 +224,6 @@ impl AtomInfo {
 fn parse_freeform<R>(
 	data: &mut R,
 	atom_len: u64,
-	reader_size: u64,
 	parse_mode: ParsingMode,
 ) -> Result<AtomIdent<'static>>
 where
@@ -237,8 +236,9 @@ where
 		err!(BadAtom("Found an incomplete freeform identifier"));
 	}
 
-	let mean = freeform_chunk(data, b"mean", reader_size, parse_mode)?;
-	let name = freeform_chunk(data, b"name", reader_size - 4, parse_mode)?;
+	let mut atom_len = atom_len;
+	let mean = freeform_chunk(data, b"mean", &mut atom_len, parse_mode)?;
+	let name = freeform_chunk(data, b"name", &mut atom_len, parse_mode)?;
 
 	Ok(AtomIdent::Freeform {
 		mean: mean.into(),
@@ -249,13 +249,13 @@ where
 fn freeform_chunk<R>(
 	data: &mut R,
 	name: &[u8],
-	reader_size: u64,
+	reader_size: &mut u64,
 	parse_mode: ParsingMode,
 ) -> Result<String>
 where
 	R: Read + Seek,
 {
-	let atom = AtomInfo::read(data, reader_size, parse_mode)?;
+	let atom = AtomInfo::read(data, *reader_size, parse_mode)?;
 
 	match atom {
 		Some(AtomInfo {
@@ -267,6 +267,10 @@ where
 				err!(BadAtom("Found an incomplete freeform identifier chunk"));
 			}
 
+			if len >= *reader_size {
+				err!(SizeMismatch);
+			}
+
 			// Version (1)
 			// Flags (3)
 			data.seek(SeekFrom::Current(4))?;
@@ -275,6 +279,8 @@ where
 			let mut content = try_vec![0; (len - 12) as usize];
 			data.read_exact(&mut content)?;
 
+			*reader_size -= len;
+
 			utf8_decode(content).map_err(|_| {
 				LoftyError::new(ErrorKind::BadAtom(
 					"Found a non UTF-8 string while reading freeform identifier",

lofty/src/mp4/ilst/read.rs

@@ -226,6 +226,18 @@ where
 			break;
 		};
 
+		if next_atom.len < 16 {
+			log::warn!(
+				"Expected data atom to be at least 16 bytes, got {}. Stopping",
+				next_atom.len
+			);
+			if parsing_mode == ParsingMode::Strict {
+				err!(BadAtom("Data atom is too small"))
+			}
+
+			break;
+		}
+
 		// We don't care about the version
 		let _version = reader.read_u8()?;
 
@@ -239,7 +251,6 @@ where
 
 		match next_atom.ident {
 			DATA_ATOM_IDENT => {
-				debug_assert!(next_atom.len >= 16);
 				let content_len = (next_atom.len - 16) as usize;
 				if content_len > 0 {
 					let mut content = try_vec![0; content_len];

lofty/src/mp4/properties.rs

@@ -244,6 +244,12 @@ where
 						mdhd = Some(atom)
 					},
 					b"hdlr" => {
+						if atom.len < 20 {
+							log::warn!("Incomplete 'hdlr' atom, skipping");
+							skip_unneeded(reader, atom.extended, atom.len)?;
+							continue;
+						}
+
 						// The hdlr atom is followed by 8 zeros
 						reader.seek(SeekFrom::Current(8))?;
 

lofty/tests/fuzz/mp4file_read_from.rs

@@ -23,3 +23,27 @@ fn panic2() {
 	);
 	let _ = Mp4File::read_from(&mut reader, ParseOptions::new());
 }
+
+#[test]
+fn panic3() {
+	let mut reader = crate::get_reader(
+		"mp4file_read_from/steam_at_mention_IDX_60_RAND_135276517902742448802109.m4a",
+	);
+	let _ = Mp4File::read_from(&mut reader, ParseOptions::new());
+}
+
+#[test]
+fn panic4() {
+	let mut reader = crate::get_reader(
+		"mp4file_read_from/steam_at_mention_IDX_83_RAND_107070306175668418039559.m4a",
+	);
+	let _ = Mp4File::read_from(&mut reader, ParseOptions::new());
+}
+
+#[test]
+fn panic5() {
+	let mut reader = crate::get_reader(
+		"mp4file_read_from/steam_at_mention_IDX_97_RAND_34488648178055098192895.m4a",
+	);
+	let _ = Mp4File::read_from(&mut reader, ParseOptions::new());
+}

lofty/tests/fuzz/wavfile_read_from.rs

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