validation improvements, fixes #301

symphonia-format-isomp4/src/atoms/avcc.rs

@@ -14,9 +14,7 @@ use symphonia_core::errors::{decode_error, Result};
 use symphonia_core::io::ReadBytes;
 
 use crate::atoms::stsd::VisualSampleEntry;
-use crate::atoms::{Atom, AtomHeader};
-
-const MAX_ATOM_SIZE: u64 = 1024;
+use crate::atoms::{Atom, AtomHeader, MAX_ATOM_SIZE};
 
 #[allow(dead_code)]
 #[derive(Debug)]

symphonia-format-isomp4/src/atoms/co64.rs

@@ -5,7 +5,7 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
-use symphonia_core::errors::Result;
+use symphonia_core::errors::{decode_error, Result};
 use symphonia_core::io::ReadBytes;
 
 use crate::atoms::{Atom, AtomHeader};
@@ -21,7 +21,17 @@ impl Atom for Co64Atom {
     fn read<B: ReadBytes>(reader: &mut B, mut header: AtomHeader) -> Result<Self> {
         let (_, _) = header.read_extended_header(reader)?;
 
+        // minimum data size is 4 bytes
+        let len = match header.data_len() {
+            Some(len) if len >= 4 => len as u32,
+            Some(_) => return decode_error("isomp4 (co64): atom size is less than 16 bytes"),
+            None => return decode_error("isomp4 (co64): expected atom size to be known"),
+        };
+
         let entry_count = reader.read_be_u32()?;
+        if entry_count != (len - 4) / 8 {
+            return decode_error("isomp4 (co64): invalid entry count");
+        }
 
         // TODO: Apply a limit.
         let mut chunk_offsets = Vec::with_capacity(entry_count as usize);

symphonia-format-isomp4/src/atoms/dac3.rs

@@ -6,11 +6,11 @@
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
 use symphonia_core::codecs::audio::well_known::CODEC_ID_AC3;
-use symphonia_core::errors::{Error, Result};
+use symphonia_core::errors::{decode_error, Result};
 use symphonia_core::io::ReadBytes;
 
 use crate::atoms::stsd::AudioSampleEntry;
-use crate::atoms::{Atom, AtomHeader};
+use crate::atoms::{Atom, AtomHeader, MAX_ATOM_SIZE};
 
 #[allow(dead_code)]
 #[derive(Debug)]
@@ -21,12 +21,13 @@ pub struct Dac3Atom {
 
 impl Atom for Dac3Atom {
     fn read<B: ReadBytes>(reader: &mut B, header: AtomHeader) -> Result<Self> {
-        // AC3SpecificBox should have length
-        let len = header
-            .data_len()
-            .ok_or(Error::DecodeError("isomp4 (dac3): expected atom size to be known"))?;
+        let len = match header.data_len() {
+            Some(len) if len <= MAX_ATOM_SIZE => len as usize,
+            Some(_) => return decode_error("isomp4 (dac3): atom size is greater than 1kb"),
+            None => return decode_error("isomp4 (dac3): expected atom size to be known"),
+        };
 
-        let extra_data = reader.read_boxed_slice_exact(len as usize)?;
+        let extra_data = reader.read_boxed_slice_exact(len)?;
 
         Ok(Dac3Atom { extra_data })
     }

symphonia-format-isomp4/src/atoms/dec3.rs

@@ -6,11 +6,11 @@
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
 use symphonia_core::codecs::audio::well_known::CODEC_ID_EAC3;
-use symphonia_core::errors::{Error, Result};
+use symphonia_core::errors::{decode_error, Result};
 use symphonia_core::io::ReadBytes;
 
 use crate::atoms::stsd::AudioSampleEntry;
-use crate::atoms::{Atom, AtomHeader};
+use crate::atoms::{Atom, AtomHeader, MAX_ATOM_SIZE};
 
 #[allow(dead_code)]
 #[derive(Debug)]
@@ -21,12 +21,13 @@ pub struct Dec3Atom {
 
 impl Atom for Dec3Atom {
     fn read<B: ReadBytes>(reader: &mut B, header: AtomHeader) -> Result<Self> {
-        // EAC3SpecificBox should have length
-        let len = header
-            .data_len()
-            .ok_or(Error::DecodeError("isomp4 (dec3): expected atom size to be known"))?;
+        let len = match header.data_len() {
+            Some(len) if len <= MAX_ATOM_SIZE => len as usize,
+            Some(_) => return decode_error("isomp4 (dec3): atom size is greater than 1kb"),
+            None => return decode_error("isomp4 (dec3): expected atom size to be known"),
+        };
 
-        let extra_data = reader.read_boxed_slice_exact(len as usize)?;
+        let extra_data = reader.read_boxed_slice_exact(len)?;
 
         Ok(Dec3Atom { extra_data })
     }

symphonia-format-isomp4/src/atoms/dovi.rs

@@ -13,8 +13,6 @@ use symphonia_core::io::ReadBytes;
 use crate::atoms::stsd::VisualSampleEntry;
 use crate::atoms::{Atom, AtomHeader};
 
-const DOVI_CONFIG_SIZE: u64 = 24;
-
 #[allow(dead_code)]
 #[derive(Debug)]
 pub struct DoviAtom {
@@ -28,7 +26,7 @@ impl Atom for DoviAtom {
         // https://professional.dolby.com/siteassets/content-creation/dolby-vision-for-content-creators/dolby_vision_bitstreams_within_the_iso_base_media_file_format_dec2017.pdf
         // It should be 24 bytes
         let len = match header.data_len() {
-            Some(len @ DOVI_CONFIG_SIZE) => len as usize,
+            Some(len @ 24) => len as usize,
             Some(_) => return decode_error("isomp4 (dvcC/dvvC): atom size is not 24 bytes"),
             None => return decode_error("isomp4 (dvcC/dvvC): expected atom size to be known"),
         };

symphonia-format-isomp4/src/atoms/elst.rs

@@ -32,9 +32,20 @@ impl Atom for ElstAtom {
     fn read<B: ReadBytes>(reader: &mut B, mut header: AtomHeader) -> Result<Self> {
         let (version, _) = header.read_extended_header(reader)?;
 
-        // TODO: Apply a limit.
+        // minimum data size is 4 bytes
+        let len = match header.data_len() {
+            Some(len) if len >= 4 => len as u32,
+            Some(_) => return decode_error("isomp4 (elst): atom size is less than 16 bytes"),
+            None => return decode_error("isomp4 (elst): expected atom size to be known"),
+        };
+
         let entry_count = reader.read_be_u32()?;
+        let entry_len = if version == 0 { 12 } else { 20 };
+        if entry_count != (len - 4) / entry_len {
+            return decode_error("isomp4 (elst): invalid entry count");
+        }
 
+        // TODO: Apply a limit.
         let mut entries = Vec::new();
 
         for _ in 0..entry_count {
@@ -47,7 +58,7 @@ impl Atom for ElstAtom {
                     reader.read_be_u64()?,
                     bits::sign_extend_leq64_to_i64(reader.read_be_u64()?, 64),
                 ),
-                _ => return decode_error("isomp4: invalid tkhd version"),
+                _ => return decode_error("isomp4 (elst): invalid version"),
             };
 
             let media_rate_int = bits::sign_extend_leq16_to_i16(reader.read_be_u16()?, 16);

symphonia-format-isomp4/src/atoms/esds.rs

@@ -43,7 +43,7 @@ fn read_descriptor_header<B: ReadBytes>(reader: &mut B) -> Result<(u8, u32)> {
 #[derive(Debug)]
 pub struct EsdsAtom {
     /// Elementary stream descriptor.
-    descriptor: ESDescriptor,
+    descriptor: Option<ESDescriptor>,
 }
 
 impl Atom for EsdsAtom {
@@ -76,28 +76,30 @@ impl Atom for EsdsAtom {
         // Ignore remainder of the atom.
         scoped.ignore()?;
 
-        Ok(EsdsAtom { descriptor: descriptor.unwrap() })
+        Ok(EsdsAtom { descriptor })
     }
 }
 
 impl EsdsAtom {
     /// If the elementary stream descriptor describes an audio stream, populate the provided
     /// audio sample entry.
     pub fn fill_audio_sample_entry(&self, entry: &mut AudioSampleEntry) -> Result<()> {
-        match get_codec_id_from_object_type(self.descriptor.dec_config.object_type_indication) {
-            Some(CodecId::Audio(id)) => {
-                // Object type indication identified an audio codec.
-                entry.codec_id = id;
-            }
-            Some(_) => {
-                // Object type indication identified a non-audio codec. This is unexpected.
-                return decode_error("isomp4 (esds): expected an audio codec type");
+        if let Some(desc) = &self.descriptor {
+            match get_codec_id_from_object_type(desc.dec_config.object_type_indication) {
+                Some(CodecId::Audio(id)) => {
+                    // Object type indication identified an audio codec.
+                    entry.codec_id = id;
+                }
+                Some(_) => {
+                    // Object type indication identified a non-audio codec. This is unexpected.
+                    return decode_error("isomp4 (esds): expected an audio codec type");
+                }
+                None => {}
             }
-            None => {}
-        }
 
-        if let Some(ds_config) = &self.descriptor.dec_config.dec_specific_info {
-            entry.extra_data = Some(ds_config.extra_data.clone());
+            if let Some(ds_config) = &desc.dec_config.dec_specific_info {
+                entry.extra_data = Some(ds_config.extra_data.clone());
+            }
         }
 
         Ok(())
@@ -106,23 +108,25 @@ impl EsdsAtom {
     /// If the elementary stream descriptor describes an video stream, populate the provided
     /// video sample entry.
     pub fn fill_video_sample_entry(&self, entry: &mut VisualSampleEntry) -> Result<()> {
-        match get_codec_id_from_object_type(self.descriptor.dec_config.object_type_indication) {
-            Some(CodecId::Video(id)) => {
-                // Object type indication identified an video codec.
-                entry.codec_id = id;
-            }
-            Some(_) => {
-                // Object type indication identified a non-video codec. This is unexpected.
-                return decode_error("isomp4 (esds): expected a video codec type");
+        if let Some(desc) = &self.descriptor {
+            match get_codec_id_from_object_type(desc.dec_config.object_type_indication) {
+                Some(CodecId::Video(id)) => {
+                    // Object type indication identified an video codec.
+                    entry.codec_id = id;
+                }
+                Some(_) => {
+                    // Object type indication identified a non-video codec. This is unexpected.
+                    return decode_error("isomp4 (esds): expected a video codec type");
+                }
+                None => {}
             }
-            None => {}
-        }
 
-        if let Some(ds_config) = &self.descriptor.dec_config.dec_specific_info {
-            entry.extra_data.push(VideoExtraData {
-                id: VIDEO_EXTRA_DATA_ID_NULL,
-                data: ds_config.extra_data.clone(),
-            });
+            if let Some(ds_config) = &desc.dec_config.dec_specific_info {
+                entry.extra_data.push(VideoExtraData {
+                    id: VIDEO_EXTRA_DATA_ID_NULL,
+                    data: ds_config.extra_data.clone(),
+                });
+            }
         }
 
         Ok(())
@@ -239,6 +243,8 @@ pub struct ESDescriptor {
 
 impl ObjectDescriptor for ESDescriptor {
     fn read<B: ReadBytes>(reader: &mut B, len: u32) -> Result<Self> {
+        let pos = reader.pos();
+
         let es_id = reader.read_be_u16()?;
         let es_flags = reader.read_u8()?;
 
@@ -261,6 +267,11 @@ impl ObjectDescriptor for ESDescriptor {
         let mut dec_config = None;
         let mut sl_config = None;
 
+        // len should be bigger than what have been read
+        if reader.pos() - pos > len as u64 {
+            return decode_error("isomp4: es descriptor len is wrong");
+        }
+
         let mut scoped = ScopedStream::new(reader, u64::from(len) - 3);
 
         // Multiple descriptors follow, but only the decoder configuration descriptor is useful.

symphonia-format-isomp4/src/atoms/ftyp.rs

@@ -29,11 +29,16 @@ impl Atom for FtypAtom {
             .ok_or(Error::DecodeError("isomp4 (ftyp): expected atom size to be known"))?;
 
         if data_len < 8 || data_len & 0x3 != 0 {
-            return decode_error("isomp4: invalid ftyp data length");
+            return decode_error("isomp4 (ftype): invalid data length");
         }
 
         // Major
-        let major = FourCc::new(reader.read_quad_bytes()?);
+        let Some(major) = FourCc::try_new(reader.read_quad_bytes()?)
+        else {
+            return decode_error(
+                "isomp4 (ftype): major - only ASCII characters are allowed in a FourCc",
+            );
+        };
 
         // Minor
         let minor = reader.read_quad_bytes()?;
@@ -44,8 +49,13 @@ impl Atom for FtypAtom {
         let mut compatible = Vec::new();
 
         for _ in 0..n_brands {
-            let brand = reader.read_quad_bytes()?;
-            compatible.push(FourCc::new(brand));
+            let Some(brand) = FourCc::try_new(reader.read_quad_bytes()?)
+            else {
+                return decode_error(
+                    "isomp4 (ftype): brand - only ASCII characters are allowed in a FourCc",
+                );
+            };
+            compatible.push(brand);
         }
 
         Ok(FtypAtom { major, minor, compatible })

symphonia-format-isomp4/src/atoms/hdlr.rs

@@ -5,7 +5,6 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
-use symphonia_core::common::FourCc;
 use symphonia_core::errors::{Error, Result};
 use symphonia_core::io::ReadBytes;
 
@@ -54,7 +53,7 @@ impl Atom for HdlrAtom {
             b"subt" => HandlerType::Subtitle,
             b"text" => HandlerType::Text,
             &hdlr => {
-                warn!("unknown handler type {:?}", FourCc::new(hdlr));
+                warn!("unknown handler type {:?}", hdlr);
                 HandlerType::Other(hdlr)
             }
         };

symphonia-format-isomp4/src/atoms/hvcc.rs

@@ -14,9 +14,7 @@ use symphonia_core::errors::{decode_error, Result};
 use symphonia_core::io::ReadBytes;
 
 use crate::atoms::stsd::VisualSampleEntry;
-use crate::atoms::{Atom, AtomHeader};
-
-const MAX_ATOM_SIZE: u64 = 1024;
+use crate::atoms::{Atom, AtomHeader, MAX_ATOM_SIZE};
 
 #[allow(dead_code)]
 #[derive(Debug)]

symphonia-format-isomp4/src/atoms/mdhd.rs

@@ -46,6 +46,11 @@ impl Atom for MdhdAtom {
     fn read<B: ReadBytes>(reader: &mut B, mut header: AtomHeader) -> Result<Self> {
         let (version, _) = header.read_extended_header(reader)?;
 
+        let expected_len = if version == 0 { 20 } else { 32 };
+        if header.data_len() != Some(expected_len) {
+            return decode_error("isomp4 (mdhd): atom size is not 32 or 44 bytes");
+        }
+
         let mut mdhd =
             MdhdAtom { ctime: 0, mtime: 0, timescale: 0, duration: 0, language: String::new() };
 
@@ -67,7 +72,7 @@ impl Atom for MdhdAtom {
                 mdhd.duration = reader.read_be_u64()?;
             }
             _ => {
-                return decode_error("isomp4: invalid mdhd version");
+                return decode_error("isomp4 (mdhd): invalid version");
             }
         }
 

symphonia-format-isomp4/src/atoms/mdia.rs

@@ -42,15 +42,15 @@ impl Atom for MdiaAtom {
         }
 
         if mdhd.is_none() {
-            return decode_error("isomp4: missing mdhd atom");
+            return decode_error("isomp4 (mdia): missing mdhd atom");
         }
 
         if hdlr.is_none() {
-            return decode_error("isomp4: missing hdlr atom");
+            return decode_error("isomp4 (mdia): missing hdlr atom");
         }
 
         if minf.is_none() {
-            return decode_error("isomp4: missing minf atom");
+            return decode_error("isomp4 (mdia): missing minf atom");
         }
 
         Ok(MdiaAtom { mdhd: mdhd.unwrap(), hdlr: hdlr.unwrap(), minf: minf.unwrap() })

symphonia-format-isomp4/src/atoms/mehd.rs

@@ -22,11 +22,16 @@ impl Atom for MehdAtom {
     fn read<B: ReadBytes>(reader: &mut B, mut header: AtomHeader) -> Result<Self> {
         let (version, _) = header.read_extended_header(reader)?;
 
+        let expected_len = if version == 0 { 4 } else { 8 };
+        if header.data_len() != Some(expected_len) {
+            return decode_error("isomp4 (mehd): atom size is not 16 or 20 bytes");
+        }
+
         let fragment_duration = match version {
             0 => u64::from(reader.read_be_u32()?),
             1 => reader.read_be_u64()?,
             _ => {
-                return decode_error("isomp4: invalid mehd version");
+                return decode_error("isomp4 (mehd): invalid version");
             }
         };