Skip to content

Commit 0720509

Browse files
baumanjbaumanj
committed
Make hdlr box parsing reject invalid input according to ParseStrictness
See #308
1 parent fca4964 commit 0720509

2 files changed

Lines changed: 136 additions & 15 deletions

File tree

‎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")
@@ -1772,7 +1778,7 @@ fn read_avif_meta<T: Read + Offset>(
17721778
"There shall be exactly one hdlr box per ISOBMFF (ISO 14496-12:2015) § 8.4.3.1",
17731779
));
17741780
}
1775-
let HandlerBox { handler_type } = read_hdlr(&mut b)?;
1781+
let HandlerBox { handler_type } = read_hdlr(&mut b, strictness)?;
17761782
if handler_type != b"pict" {
17771783
fail_if(
17781784
strictness != ParseStrictness::Permissive,
@@ -1814,7 +1820,7 @@ fn read_avif_meta<T: Read + Offset>(
18141820
}
18151821
BoxType::ItemPropertiesBox => {
18161822
if item_properties.is_some() {
1817-
return Err(Error::InvalidData("There shall be zero or one iprp boxes per ISOBMFF (ISO 14496-12:2020 § 8.11.14.1"));
1823+
return Err(Error::InvalidData("There shall be zero or one iprp boxes per ISOBMFF (ISO 14496-12:2020) § 8.11.14.1"));
18181824
}
18191825
item_properties = Some(read_iprp(&mut b, MIF1_BRAND, strictness)?);
18201826
}
@@ -3081,7 +3087,7 @@ fn read_mdia<T: Read>(f: &mut BMFFBox<T>, track: &mut Track) -> Result<()> {
30813087
debug!("{:?}", mdhd);
30823088
}
30833089
BoxType::HandlerBox => {
3084-
let hdlr = read_hdlr(&mut b)?;
3090+
let hdlr = read_hdlr(&mut b, ParseStrictness::Permissive)?;
30853091

30863092
match hdlr.handler_type.value.as_ref() {
30873093
b"vide" => track.track_type = TrackType::Video,
@@ -4088,20 +4094,50 @@ fn read_alac<T: Read>(src: &mut BMFFBox<T>) -> Result<ALACSpecificBox> {
40884094
}
40894095

40904096
/// Parse a Handler Reference Box.
4091-
/// See ISOBMFF (ISO 14496-12:2015) § 8.4.3
4092-
fn read_hdlr<T: Read>(src: &mut BMFFBox<T>) -> Result<HandlerBox> {
4093-
let (_, _) = read_fullbox_extra(src)?;
4097+
/// See ISOBMFF (ISO 14496-12:2020) § 8.4.3
4098+
fn read_hdlr<T: Read>(src: &mut BMFFBox<T>, strictness: ParseStrictness) -> Result<HandlerBox> {
4099+
if read_fullbox_version_no_flags(src)? != 0 {
4100+
return Err(Error::Unsupported("hdlr version"));
4101+
}
40944102

4095-
// Skip uninteresting fields.
4096-
skip(src, 4)?;
4103+
let pre_defined = be_u32(src)?;
4104+
if pre_defined != 0 {
4105+
fail_if(
4106+
strictness != ParseStrictness::Permissive,
4107+
"The HandlerBox 'pre_defined' field shall be 0 \
4108+
per ISOBMFF (ISO 14496-12:2020) § 8.4.3.2",
4109+
)?;
4110+
}
40974111

40984112
let handler_type = FourCC::from(be_u32(src)?);
40994113

4100-
// Skip uninteresting fields.
4101-
skip(src, 12)?;
4114+
for _ in 1..=3 {
4115+
let reserved = be_u32(src)?;
4116+
if reserved != 0 {
4117+
fail_if(
4118+
strictness != ParseStrictness::Permissive,
4119+
"The HandlerBox 'reserved' fields shall be 0 \
4120+
per ISOBMFF (ISO 14496-12:2020) § 8.4.3.2",
4121+
)?;
4122+
}
4123+
}
41024124

4103-
// Skip name.
4104-
skip_box_remain(src)?;
4125+
match std::str::from_utf8(src.read_into_try_vec()?.as_slice()) {
4126+
Ok(name) => {
4127+
if name.bytes().last() != Some(b'\0') {
4128+
fail_if(
4129+
strictness != ParseStrictness::Permissive,
4130+
"The HandlerBox 'name' field shall be null-terminated \
4131+
per ISOBMFF (ISO 14496-12:2020) § 8.4.3.2",
4132+
)?
4133+
}
4134+
}
4135+
Err(_) => fail_if(
4136+
strictness != ParseStrictness::Permissive,
4137+
"The HandlerBox 'name' field shall be valid utf8 \
4138+
per ISOBMFF (ISO 14496-12:2020) § 8.4.3.2",
4139+
)?,
4140+
}
41054141

41064142
Ok(HandlerBox { handler_type })
41074143
}

‎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)