Allow skipped top-level boxes to extend past EOF.

This allows files with garbage appended to the end to still load.

Author: Zaggy1024

mp4parse/src/lib.rs

@@ -2273,11 +2273,11 @@ impl<'a, T: Read> BoxIter<'a, T> {
     fn next_box(&mut self) -> Result<Option<BMFFBox<T>>> {
         let r = read_box_header(self.src);
         match r {
-            Ok(h) => Ok(Some(BMFFBox {
+            Ok(Some(h)) => Ok(Some(BMFFBox {
                 head: h,
                 content: self.src.take(h.size.saturating_sub(h.offset)),
             })),
-            Err(Error::UnexpectedEOF) => Ok(None),
+            Ok(None) => Ok(None),
             Err(e) => Err(e),
         }
     }
@@ -2332,8 +2332,12 @@ impl<'a, T> Drop for BMFFBox<'a, T> {
 /// skip unknown or uninteresting boxes.
 ///
 /// See ISOBMFF (ISO 14496-12:2020) § 4.2
-fn read_box_header<T: ReadBytesExt>(src: &mut T) -> Result<BoxHeader> {
-    let size32 = be_u32(src)?;
+fn read_box_header<T: ReadBytesExt>(src: &mut T) -> Result<Option<BoxHeader>> {
+    let size32 = match be_u32(src) {
+        Ok(v) => v,
+        Err(Error::UnexpectedEOF) => return Ok(None),
+        Err(error) => return Err(error),
+    };
     let name = BoxType::from(be_u32(src)?);
     let size = match size32 {
         // valid only for top-level box and indicates it's the last box in the file.  usually mdat.
@@ -2382,12 +2386,12 @@ fn read_box_header<T: ReadBytesExt>(src: &mut T) -> Result<BoxHeader> {
         None
     };
     assert!(offset <= size || size == 0);
-    Ok(BoxHeader {
+    Ok(Some(BoxHeader {
         name,
         size,
         offset,
         uuid,
-    })
+    }))
 }
 
 /// Parse the extra header fields for a full box.
@@ -2521,7 +2525,19 @@ pub fn read_avif<T: Read>(f: &mut T, strictness: ParseStrictness) -> Result<Avif
     let mut image_sequence = None;
     let mut media_storage = TryVec::new();
 
-    while let Some(mut b) = iter.next_box()? {
+    loop {
+        let mut b = match iter.next_box() {
+            Ok(Some(b)) => b,
+            Ok(_) => break,
+            Err(Error::UnexpectedEOF) => {
+                if strictness == ParseStrictness::Strict {
+                    return Err(Error::UnexpectedEOF);
+                }
+                break;
+            }
+            Err(error) => return Err(error),
+        };
+
         trace!("read_avif parsing {:?} box", b.head.name);
         match b.head.name {
             BoxType::MetadataBox => {
@@ -2564,7 +2580,14 @@ pub fn read_avif<T: Read>(f: &mut T, strictness: ParseStrictness) -> Result<Avif
                 };
                 media_storage.push(DataBox::from_mdat(file_offset, data))?;
             }
-            _ => skip_box_content(&mut b)?,
+            _ => {
+                let result = skip_box_content(&mut b);
+                // Allow garbage at EOF if we aren't in strict mode.
+                if b.bytes_left() > 0 && strictness != ParseStrictness::Strict {
+                    break;
+                }
+                result?;
+            }
         }
 
         check_parser_state!(b.content);

mp4parse/src/tests.rs

@@ -93,7 +93,7 @@ where
 #[test]
 fn read_box_header_short() {
     let mut stream = make_box(BoxSize::Short(8), b"test", |s| s);
-    let header = super::read_box_header(&mut stream).unwrap();
+    let header = super::read_box_header(&mut stream).unwrap().unwrap();
     assert_eq!(header.name, BoxType::UnknownBox(0x7465_7374)); // "test"
     assert_eq!(header.size, 8);
     assert!(header.uuid.is_none());
@@ -102,7 +102,7 @@ fn read_box_header_short() {
 #[test]
 fn read_box_header_long() {
     let mut stream = make_box(BoxSize::Long(16), b"test", |s| s);
-    let header = super::read_box_header(&mut stream).unwrap();
+    let header = super::read_box_header(&mut stream).unwrap().unwrap();
     assert_eq!(header.name, BoxType::UnknownBox(0x7465_7374)); // "test"
     assert_eq!(header.size, 16);
     assert!(header.uuid.is_none());

mp4parse/tests/public.rs

@@ -68,6 +68,9 @@ static IMAGE_AVIF_CLAP_MISSING_ESSENTIAL: &str = "tests/clap-missing-essential.a
 static IMAGE_AVIF_UNKNOWN_MDAT_SIZE: &str = "tests/unknown_mdat.avif";
 static IMAGE_AVIF_UNKNOWN_MDAT_SIZE_IN_OVERSIZED_META: &str =
     "tests/unknown_mdat_in_oversized_meta.avif";
+static IMAGE_AVIF_VALID_WITH_GARBAGE_OVERREAD_AT_END: &str =
+    "tests/valid_with_garbage_overread.avif";
+static IMAGE_AVIF_VALID_WITH_GARBAGE_BYTE_AT_END: &str = "tests/valid_with_garbage_byte.avif";
 static AVIF_TEST_DIRS: &[&str] = &["tests", "av1-avif/testFiles", "link-u-avif-sample-images"];
 
 // These files are
@@ -1279,6 +1282,19 @@ fn public_avif_avis_with_pitm_no_iloc() {
     assert_avif_should(AVIF_AVIS_WITH_PITM_NO_ILOC, Status::PitmNotFound);
 }
 
+#[test]
+fn public_avif_valid_with_garbage_overread_at_end() {
+    assert_avif_should(
+        IMAGE_AVIF_VALID_WITH_GARBAGE_OVERREAD_AT_END,
+        Status::CheckParserStateErr,
+    );
+}
+
+#[test]
+fn public_avif_valid_with_garbage_byte_at_end() {
+    assert_avif_should(IMAGE_AVIF_VALID_WITH_GARBAGE_BYTE_AT_END, Status::Eof);
+}
+
 fn public_avis_loop_impl(path: &str, looped: bool) {
     let input = &mut File::open(path).expect("Unknown file");
     match mp4::read_avif(input, ParseStrictness::Normal) {