MP4: Fix panic with improperly sized freeform idents

Author: Serial-ATA

CHANGELOG.md

@@ -25,7 +25,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 - **Fuzzing** (Thanks [@qarmin](https://github.com/qarmin)!) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/TODO)):
-  - **MP4**: Fix panic when reading properties of a file with no timescale specified ([issue](https://github.com/Serial-ATA/lofty-rs/issues/418))
+  - **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))
   - **WAV**: Fix panic when reading properties with large written bytes per second ([issue](https://github.com/Serial-ATA/lofty-rs/issues/420))
   - **Vorbis**: Fix panic when reading properties of a file with large absolute granule positions ([issue](https://github.com/Serial-ATA/lofty-rs/issues/421))
   - **FLAC**: Fix panic when reading properties of a file with incorrect block sizes ([issue](https://github.com/Serial-ATA/lofty-rs/issues/422))

lofty/src/mp4/atom_info.rs

@@ -192,7 +192,7 @@ impl AtomInfo {
 				err!(BadAtom("Found an incomplete freeform identifier"));
 			}
 
-			atom_ident = parse_freeform(data, reader_size, parse_mode)?;
+			atom_ident = parse_freeform(data, len, reader_size, parse_mode)?;
 		} else {
 			atom_ident = AtomIdent::Fourcc(identifier);
 		}
@@ -208,12 +208,20 @@ impl AtomInfo {
 
 fn parse_freeform<R>(
 	data: &mut R,
+	atom_len: u64,
 	reader_size: u64,
 	parse_mode: ParsingMode,
 ) -> Result<AtomIdent<'static>>
 where
 	R: Read + Seek,
 {
+	// ---- header + mean header + name header = 24
+	const MINIMUM_FREEFORM_LEN: u64 = ATOM_HEADER_LEN * 3;
+
+	if atom_len < MINIMUM_FREEFORM_LEN {
+		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)?;
 
@@ -240,11 +248,15 @@ where
 			len,
 			..
 		}) if fourcc == name => {
+			if len < 12 {
+				err!(BadAtom("Found an incomplete freeform identifier chunk"));
+			}
+
 			// Version (1)
 			// Flags (3)
 			data.seek(SeekFrom::Current(4))?;
 
-			// Already read the size, identifier, and version/flags (12 bytes)
+			// Already read the size (4) + identifier (4) + version/flags (4)
 			let mut content = try_vec![0; (len - 12) as usize];
 			data.read_exact(&mut content)?;
 

lofty/src/mp4/ilst/read.rs

@@ -212,7 +212,14 @@ where
 	// An atom can contain multiple data atoms
 	let mut ret = Vec::new();
 
-	let to_read = (atom_info.start + atom_info.len) - reader.stream_position()?;
+	let atom_end = atom_info.start + atom_info.len;
+	let position = reader.stream_position()?;
+	assert!(
+		atom_end >= position,
+		"uncaught size mismatch, reader position: {position} (expected <= {atom_end})",
+	);
+
+	let to_read = atom_end - position;
 	let mut pos = 0;
 	while pos < to_read {
 		let Some(next_atom) = reader.next()? else {

lofty/tests/fuzz/assets/mp4file_read_from/steam_at_mention_IDX_33_RAND_122808229373977607781108.m4a

@@ -15,3 +15,11 @@ fn panic1() {
 	);
 	let _ = Mp4File::read_from(&mut reader, ParseOptions::new());
 }
+
+#[test]
+fn panic2() {
+	let mut reader = crate::get_reader(
+		"mp4file_read_from/steam_at_mention_IDX_33_RAND_122808229373977607781108.m4a",
+	);
+	let _ = Mp4File::read_from(&mut reader, ParseOptions::new());
+}