Merge pull request mozilla#256 from mozilla/ipma-validate-moar

Add one last ipma validity check

Author: baumanj

mp4parse/src/lib.rs

@@ -1758,12 +1758,15 @@ fn read_iprp<T: Read>(src: &mut BMFFBox<T>) -> Result<TryVec<AssociatedProperty>
             return Err(Error::InvalidData("unexpected iprp child"));
         }
 
-        let version_and_flags = read_fullbox_extra(&mut b)?;
-        if ipma_version_and_flag_values_seen.contains(&version_and_flags) {
+        let (version, flags) = read_fullbox_extra(&mut b)?;
+        if ipma_version_and_flag_values_seen.contains(&(version, flags)) {
             return Err(Error::InvalidData("Duplicate ipma with same version/flags"));
         }
-        ipma_version_and_flag_values_seen.push(version_and_flags)?;
-        let associations = read_ipma(&mut b, version_and_flags)?;
+        if flags != 0 && properties.len() <= 127 {
+            return Err(Error::InvalidData("flags should be equal to 0 unless there are more than 127 properties in the ItemPropertyContainerBox"));
+        }
+        ipma_version_and_flag_values_seen.push((version, flags))?;
+        let associations = read_ipma(&mut b, version, flags)?;
         for a in associations {
             if a.property_index == 0 {
                 if a.essential {
@@ -1965,7 +1968,8 @@ fn calculate_ipma_total_associations(
 /// See HEIF (ISO 23008-12:2017) § 9.3.1
 fn read_ipma<T: Read>(
     src: &mut BMFFBox<T>,
-    (version, flags): (u8, u32),
+    version: u8,
+    flags: u32,
 ) -> Result<TryVec<Association>> {
     let entry_count = be_u32(src)?;
     let num_association_bytes =

mp4parse/tests/corrupt/bad-ipma-flags.avif

@@ -33,6 +33,7 @@ static IMAGE_AVIF_EXTENTS: &str = "tests/kodim-extents.avif";
 static IMAGE_AVIF_CORRUPT: &str = "tests/corrupt/bug-1655846.avif";
 static IMAGE_AVIF_CORRUPT_2: &str = "tests/corrupt/bug-1661347.avif";
 static IMAGE_AVIF_CORRUPT_3: &str = "tests/corrupt/bad-ipma-version.avif";
+static IMAGE_AVIF_CORRUPT_4: &str = "tests/corrupt/bad-ipma-flags.avif";
 static IMAGE_AVIF_GRID: &str = "av1-avif/testFiles/Microsoft/Summer_in_Tomsk_720p_5x4_grid.avif";
 static AVIF_TEST_DIRS: &[&str] = &["tests", "av1-avif/testFiles"];
 static AVIF_CORRUPT_IMAGES: &str = "tests/corrupt";
@@ -643,11 +644,8 @@ fn public_avif_bug_1661347() {
     assert!(mp4::read_avif(input).is_err());
 }
 
-#[test]
-fn public_avif_bad_ipma_version() {
-    let input = &mut File::open(IMAGE_AVIF_CORRUPT_3).expect("Unknown file");
-    let expected_msg = "The version 0 should be used unless 32-bit item_ID values are needed";
-    match mp4::read_avif(input) {
+fn assert_invalid_data<T: std::fmt::Debug>(result: mp4::Result<T>, expected_msg: &str) {
+    match result {
         Err(Error::InvalidData(msg)) if msg == expected_msg => {}
         r => panic!(
             "Expected Err(Error::InvalidData({:?})), found {:?}",
@@ -656,6 +654,20 @@ fn public_avif_bad_ipma_version() {
     }
 }
 
+#[test]
+fn public_avif_bad_ipma_version() {
+    let input = &mut File::open(IMAGE_AVIF_CORRUPT_3).expect("Unknown file");
+    let expected_msg = "The version 0 should be used unless 32-bit item_ID values are needed";
+    assert_invalid_data(mp4::read_avif(input), expected_msg);
+}
+
+#[test]
+fn public_avif_bad_ipma_flags() {
+    let input = &mut File::open(IMAGE_AVIF_CORRUPT_4).expect("Unknown file");
+    let expected_msg = "flags should be equal to 0 unless there are more than 127 properties in the ItemPropertyContainerBox";
+    assert_invalid_data(mp4::read_avif(input), expected_msg);
+}
+
 #[test]
 #[ignore] // Remove when we add support; see https://github.com/mozilla/mp4parse-rust/issues/198
 fn public_avif_primary_item_is_grid() {