Skip to content

Commit 1c5dc60

Browse files
baumanjbaumanj
authored
Merge pull request #309 from baumanj/hdlr-compliance
Make hdlr box parsing reject invalid input according to ParseStrictness
2 parents 6493e28 + 0720509 commit 1c5dc60

File tree

2 files changed

+136
-15
lines changed

2 files changed

+136
-15
lines changed

‎mp4parse/src/lib.rs

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,12 @@ impl From<std::string::FromUtf8Error> for Error {
204204
}
205205
}
206206

207+
impl From<std::str::Utf8Error> for Error {
208+
fn from(_: std::str::Utf8Error) -> Error {
209+
Error::InvalidData("invalid utf8")
210+
}
211+
}
212+
207213
impl From<std::num::TryFromIntError> for Error {
208214
fn from(_: std::num::TryFromIntError) -> Error {
209215
Error::Unsupported("integer conversion failed")
@@ -1774,7 +1780,7 @@ fn read_avif_meta<T: Read + Offset>(
17741780
"There shall be exactly one hdlr box per ISOBMFF (ISO 14496-12:2015) § 8.4.3.1",
17751781
));
17761782
}
1777-
let HandlerBox { handler_type } = read_hdlr(&mut b)?;
1783+
let HandlerBox { handler_type } = read_hdlr(&mut b, strictness)?;
17781784
if handler_type != b"pict" {
17791785
fail_if(
17801786
strictness != ParseStrictness::Permissive,
@@ -1816,7 +1822,7 @@ fn read_avif_meta<T: Read + Offset>(
18161822
}
18171823
BoxType::ItemPropertiesBox => {
18181824
if item_properties.is_some() {
1819-
return Err(Error::InvalidData("There shall be zero or one iprp boxes per ISOBMFF (ISO 14496-12:2020 § 8.11.14.1"));
1825+
return Err(Error::InvalidData("There shall be zero or one iprp boxes per ISOBMFF (ISO 14496-12:2020) § 8.11.14.1"));
18201826
}
18211827
item_properties = Some(read_iprp(&mut b, MIF1_BRAND, strictness)?);
18221828
}
@@ -3099,7 +3105,7 @@ fn read_mdia<T: Read>(f: &mut BMFFBox<T>, track: &mut Track) -> Result<()> {
30993105
debug!("{:?}", mdhd);
31003106
}
31013107
BoxType::HandlerBox => {
3102-
let hdlr = read_hdlr(&mut b)?;
3108+
let hdlr = read_hdlr(&mut b, ParseStrictness::Permissive)?;
31033109

31043110
match hdlr.handler_type.value.as_ref() {
31053111
b"vide" => track.track_type = TrackType::Video,
@@ -4122,20 +4128,50 @@ fn read_alac<T: Read>(src: &mut BMFFBox<T>) -> Result<ALACSpecificBox> {
41224128
}
41234129

41244130
/// Parse a Handler Reference Box.
4125-
/// See ISOBMFF (ISO 14496-12:2015) § 8.4.3
4126-
fn read_hdlr<T: Read>(src: &mut BMFFBox<T>) -> Result<HandlerBox> {
4127-
let (_, _) = read_fullbox_extra(src)?;
4131+
/// See ISOBMFF (ISO 14496-12:2020) § 8.4.3
4132+
fn read_hdlr<T: Read>(src: &mut BMFFBox<T>, strictness: ParseStrictness) -> Result<HandlerBox> {
4133+
if read_fullbox_version_no_flags(src)? != 0 {
4134+
return Err(Error::Unsupported("hdlr version"));
4135+
}
41284136

4129-
// Skip uninteresting fields.
4130-
skip(src, 4)?;
4137+
let pre_defined = be_u32(src)?;
4138+
if pre_defined != 0 {
4139+
fail_if(
4140+
strictness != ParseStrictness::Permissive,
4141+
"The HandlerBox 'pre_defined' field shall be 0 \
4142+
per ISOBMFF (ISO 14496-12:2020) § 8.4.3.2",
4143+
)?;
4144+
}
41314145

41324146
let handler_type = FourCC::from(be_u32(src)?);
41334147

4134-
// Skip uninteresting fields.
4135-
skip(src, 12)?;
4148+
for _ in 1..=3 {
4149+
let reserved = be_u32(src)?;
4150+
if reserved != 0 {
4151+
fail_if(
4152+
strictness != ParseStrictness::Permissive,
4153+
"The HandlerBox 'reserved' fields shall be 0 \
4154+
per ISOBMFF (ISO 14496-12:2020) § 8.4.3.2",
4155+
)?;
4156+
}
4157+
}
41364158

4137-
// Skip name.
4138-
skip_box_remain(src)?;
4159+
match std::str::from_utf8(src.read_into_try_vec()?.as_slice()) {
4160+
Ok(name) => {
4161+
if name.bytes().last() != Some(b'\0') {
4162+
fail_if(
4163+
strictness != ParseStrictness::Permissive,
4164+
"The HandlerBox 'name' field shall be null-terminated \
4165+
per ISOBMFF (ISO 14496-12:2020) § 8.4.3.2",
4166+
)?
4167+
}
4168+
}
4169+
Err(_) => fail_if(
4170+
strictness != ParseStrictness::Permissive,
4171+
"The HandlerBox 'name' field shall be valid utf8 \
4172+
per ISOBMFF (ISO 14496-12:2020) § 8.4.3.2",
4173+
)?,
4174+
}
41394175

41404176
Ok(HandlerBox { handler_type })
41414177
}

‎mp4parse/src/tests.rs

Lines changed: 88 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
use super::read_mp4;
99
use super::Error;
10+
use super::ParseStrictness;
1011
use fallible_collections::TryRead as _;
1112

1213
use std::convert::TryInto as _;
@@ -470,7 +471,7 @@ fn read_hdlr() {
470471
let mut stream = iter.next_box().unwrap().unwrap();
471472
assert_eq!(stream.head.name, BoxType::HandlerBox);
472473
assert_eq!(stream.head.size, 45);
473-
let parsed = super::read_hdlr(&mut stream).unwrap();
474+
let parsed = super::read_hdlr(&mut stream, ParseStrictness::Normal).unwrap();
474475
assert_eq!(parsed.handler_type, b"vide");
475476
}
476477

@@ -483,10 +484,72 @@ fn read_hdlr_short_name() {
483484
let mut stream = iter.next_box().unwrap().unwrap();
484485
assert_eq!(stream.head.name, BoxType::HandlerBox);
485486
assert_eq!(stream.head.size, 33);
486-
let parsed = super::read_hdlr(&mut stream).unwrap();
487+
let parsed = super::read_hdlr(&mut stream, ParseStrictness::Normal).unwrap();
487488
assert_eq!(parsed.handler_type, b"vide");
488489
}
489490

491+
#[test]
492+
fn read_hdlr_unsupported_version() {
493+
let mut stream = make_fullbox(BoxSize::Short(32), b"hdlr", 1, |s| {
494+
s.B32(0).append_bytes(b"vide").B32(0).B32(0).B32(0)
495+
});
496+
let mut iter = super::BoxIter::new(&mut stream);
497+
let mut stream = iter.next_box().unwrap().unwrap();
498+
assert_eq!(stream.head.name, BoxType::HandlerBox);
499+
assert_eq!(stream.head.size, 32);
500+
match super::read_hdlr(&mut stream, ParseStrictness::Normal) {
501+
Err(Error::Unsupported(msg)) => assert_eq!("hdlr version", msg),
502+
result => {
503+
eprintln!("{:?}", result);
504+
panic!("expected Error::Unsupported")
505+
}
506+
}
507+
}
508+
509+
#[test]
510+
fn read_hdlr_invalid_pre_defined_field() {
511+
let mut stream = make_fullbox(BoxSize::Short(32), b"hdlr", 0, |s| {
512+
s.B32(1).append_bytes(b"vide").B32(0).B32(0).B32(0)
513+
});
514+
let mut iter = super::BoxIter::new(&mut stream);
515+
let mut stream = iter.next_box().unwrap().unwrap();
516+
assert_eq!(stream.head.name, BoxType::HandlerBox);
517+
assert_eq!(stream.head.size, 32);
518+
match super::read_hdlr(&mut stream, ParseStrictness::Normal) {
519+
Err(Error::InvalidData(msg)) => assert_eq!(
520+
"The HandlerBox 'pre_defined' field shall be 0 \
521+
per ISOBMFF (ISO 14496-12:2020) § 8.4.3.2",
522+
msg
523+
),
524+
result => {
525+
eprintln!("{:?}", result);
526+
panic!("expected Error::InvalidData")
527+
}
528+
}
529+
}
530+
531+
#[test]
532+
fn read_hdlr_invalid_reserved_field() {
533+
let mut stream = make_fullbox(BoxSize::Short(32), b"hdlr", 0, |s| {
534+
s.B32(0).append_bytes(b"vide").B32(0).B32(1).B32(0)
535+
});
536+
let mut iter = super::BoxIter::new(&mut stream);
537+
let mut stream = iter.next_box().unwrap().unwrap();
538+
assert_eq!(stream.head.name, BoxType::HandlerBox);
539+
assert_eq!(stream.head.size, 32);
540+
match super::read_hdlr(&mut stream, ParseStrictness::Normal) {
541+
Err(Error::InvalidData(msg)) => assert_eq!(
542+
"The HandlerBox 'reserved' fields shall be 0 \
543+
per ISOBMFF (ISO 14496-12:2020) § 8.4.3.2",
544+
msg
545+
),
546+
result => {
547+
eprintln!("{:?}", result);
548+
panic!("expected Error::InvalidData")
549+
}
550+
}
551+
}
552+
490553
#[test]
491554
fn read_hdlr_zero_length_name() {
492555
let mut stream = make_fullbox(BoxSize::Short(32), b"hdlr", 0, |s| {
@@ -496,7 +559,29 @@ fn read_hdlr_zero_length_name() {
496559
let mut stream = iter.next_box().unwrap().unwrap();
497560
assert_eq!(stream.head.name, BoxType::HandlerBox);
498561
assert_eq!(stream.head.size, 32);
499-
let parsed = super::read_hdlr(&mut stream).unwrap();
562+
match super::read_hdlr(&mut stream, ParseStrictness::Normal) {
563+
Err(Error::InvalidData(msg)) => assert_eq!(
564+
"The HandlerBox 'name' field shall be null-terminated \
565+
per ISOBMFF (ISO 14496-12:2020) § 8.4.3.2",
566+
msg
567+
),
568+
result => {
569+
eprintln!("{:?}", result);
570+
panic!("expected Error::InvalidData")
571+
}
572+
}
573+
}
574+
575+
#[test]
576+
fn read_hdlr_zero_length_name_permissive() {
577+
let mut stream = make_fullbox(BoxSize::Short(32), b"hdlr", 0, |s| {
578+
s.B32(0).append_bytes(b"vide").B32(0).B32(0).B32(0)
579+
});
580+
let mut iter = super::BoxIter::new(&mut stream);
581+
let mut stream = iter.next_box().unwrap().unwrap();
582+
assert_eq!(stream.head.name, BoxType::HandlerBox);
583+
assert_eq!(stream.head.size, 32);
584+
let parsed = super::read_hdlr(&mut stream, ParseStrictness::Permissive).unwrap();
500585
assert_eq!(parsed.handler_type, b"vide");
501586
}
502587

0 commit comments

Comments
 (0)