OGG: Add length validation to Vorbis Comments reading

CHANGELOG.md

@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **PictureInformation**: Fix potential overflow on an invalid picture
 - **MP4**: The parser has received a major facelift, and shouldn't be so eager to allocate or trust user data (Fixes OOM)
 - **FLAC**: Return early when encountering invalid zero-sized blocks
+- **FLAC/Opus/Vorbis/Speex**: Add better length validity checks while reading Vorbis Comments (Fixes OOM)
 
 ## [0.7.1] - 2022-07-08
 

src/flac/read.rs

@@ -97,7 +97,7 @@ where
 
 		match block.ty {
 			#[cfg(feature = "vorbis_comments")]
-			4 => read_comments(&mut &*block.content, &mut tag)?,
+			4 => read_comments(&mut &*block.content, block.content.len() as u64, &mut tag)?,
 			#[cfg(feature = "vorbis_comments")]
 			6 => tag
 				.pictures

src/ogg/read.rs

@@ -19,7 +19,7 @@ pub type OGGTags = (Option<VorbisComments>, Page);
 pub type OGGTags = (Option<()>, Page);
 
 #[cfg(feature = "vorbis_comments")]
-pub(crate) fn read_comments<R>(data: &mut R, tag: &mut VorbisComments) -> Result<()>
+pub(crate) fn read_comments<R>(data: &mut R, mut len: u64, tag: &mut VorbisComments) -> Result<()>
 where
 	R: Read,
 {
@@ -27,10 +27,15 @@ where
 	use crate::macros::try_vec;
 
 	let vendor_len = data.read_u32::<LittleEndian>()?;
+	if u64::from(vendor_len) > len {
+		err!(TooMuchData);
+	}
 
 	let mut vendor = try_vec![0; vendor_len as usize];
 	data.read_exact(&mut vendor)?;
 
+	len -= u64::from(vendor_len);
+
 	let vendor = match String::from_utf8(vendor) {
 		Ok(v) => v,
 		Err(e) => {
@@ -61,10 +66,16 @@ where
 
 	for _ in 0..comments_total_len {
 		let comment_len = data.read_u32::<LittleEndian>()?;
+		if u64::from(comment_len) > len {
+			// TODO: Maybe add ErrorKind::SizeMismatch?
+			err!(TooMuchData);
+		}
 
 		let mut comment_bytes = try_vec![0; comment_len as usize];
 		data.read_exact(&mut comment_bytes)?;
 
+		len -= u64::from(comment_len);
+
 		let comment = String::from_utf8(comment_bytes)?;
 		let mut comment_split = comment.splitn(2, '=');
 
@@ -123,7 +134,7 @@ where
 		let mut tag = VorbisComments::default();
 
 		let reader = &mut &md_pages[..];
-		read_comments(reader, &mut tag)?;
+		read_comments(reader, reader.len() as u64, &mut tag)?;
 
 		Ok((Some(tag), first_page))
 	}

src/ogg/tag.rs

@@ -554,7 +554,7 @@ mod tests {
 		let mut reader = std::io::Cursor::new(tag);
 		let mut parsed_tag = VorbisComments::default();
 
-		crate::ogg::read::read_comments(&mut reader, &mut parsed_tag).unwrap();
+		crate::ogg::read::read_comments(&mut reader, tag.len() as u64, &mut parsed_tag).unwrap();
 		parsed_tag
 	}
 
@@ -597,11 +597,7 @@ mod tests {
 	#[test]
 	fn vorbis_comments_to_tag() {
 		let tag_bytes = std::fs::read("tests/tags/assets/test.vorbis").unwrap();
-
-		let mut reader = std::io::Cursor::new(&tag_bytes[..]);
-		let mut vorbis_comments = VorbisComments::default();
-
-		crate::ogg::read::read_comments(&mut reader, &mut vorbis_comments).unwrap();
+		let vorbis_comments = read_tag(&tag_bytes);
 
 		let tag: Tag = vorbis_comments.into();
 
@@ -625,10 +621,6 @@ mod tests {
 	#[test]
 	fn zero_sized_vorbis_comments() {
 		let tag_bytes = std::fs::read("tests/tags/assets/zero.vorbis").unwrap();
-
-		let mut reader = std::io::Cursor::new(&tag_bytes[..]);
-		let mut vorbis_comments = VorbisComments::default();
-
-		crate::ogg::read::read_comments(&mut reader, &mut vorbis_comments).unwrap();
+		let _ = read_tag(&tag_bytes);
 	}
 }

tests/fuzz/opusfile_read_from.rs

@@ -1 +1,7 @@
-// TODO
+use crate::oom_test;
+use lofty::ogg::OpusFile;
+
+#[test]
+fn oom1() {
+	oom_test::<OpusFile>("opusfile_read_from/oom-7126e68a5a9ef53351c46f3c55b7e1a582705fcc");
+}

tests/fuzz/speexfile_read_from.rs

@@ -1 +1,7 @@
-// TODO
+use crate::oom_test;
+use lofty::ogg::SpeexFile;
+
+#[test]
+fn oom1() {
+	oom_test::<SpeexFile>("speexfile_read_from/oom-7976a4c57e7f8b4ac428f9e7f846b59d2dce714f");
+}

tests/fuzz/vorbisfile_read_from.rs

@@ -1 +1,7 @@
-// TODO
+use crate::oom_test;
+use lofty::ogg::VorbisFile;
+
+#[test]
+fn oom1() {
+	oom_test::<VorbisFile>("vorbisfile_read_from/oom-436193bc2d1664b74c19720bef08697d03284f06");
+}