From 8c2560f31cba86def42f779b2d396d6d37253061 Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Wed, 28 Oct 2026 15:06:23 -0500 Subject: [PATCH 1/5] codec: validate configuration NAL types --- crates/h264/src/decoder.rs | 35 +++++++++++++++++++++++++++++----- crates/h264/src/synth_tests.rs | 14 ++++++++++++++ crates/hevc/src/decoder.rs | 31 ++++++++++++++++++++++++++++-- crates/hevc/src/synth_tests.rs | 15 +++++++++++++++ 4 files changed, 88 insertions(+), 7 deletions(-) diff --git a/crates/h264/src/decoder.rs b/crates/h264/src/decoder.rs index 37ef1f299..64fcb48b1 100644 --- a/crates/h264/src/decoder.rs +++ b/crates/h264/src/decoder.rs @@ -216,26 +216,37 @@ impl Decoder { pub fn configure_avcc(&mut self, avcc: &[u8]) -> Result<()> { ensure!(avcc.len() >= 7, "avcC too short"); ensure!(avcc[0] == 1, "unsupported avcC version {}", avcc[0]); - self.nal_length_size = Some((avcc[4] & 3) as usize + 1); let mut pos = 5; - let read_sets = |pos: &mut usize, count: usize, d: &mut Decoder| -> Result<()> { + let mut sets = Vec::new(); + let read_sets = |pos: &mut usize, count: usize, sets: &mut Vec<(usize, usize)>| -> Result<()> { for _ in 0..count { ensure!(*pos + 2 <= avcc.len(), "avcC truncated"); let len = u16::from_be_bytes([avcc[*pos], avcc[*pos + 1]]) as usize; *pos += 2; ensure!(*pos + len <= avcc.len(), "avcC truncated"); - d.handle_nal(&avcc[*pos..*pos + len], 0)?; + sets.push((*pos, len)); *pos += len; } Ok(()) }; let nsps = (avcc[pos] & 0x1f) as usize; pos += 1; - read_sets(&mut pos, nsps, self)?; + read_sets(&mut pos, nsps, &mut sets)?; ensure!(pos < avcc.len(), "avcC truncated"); let npps = avcc[pos] as usize; pos += 1; - read_sets(&mut pos, npps, self) + read_sets(&mut pos, npps, &mut sets)?; + for (i, (start, len)) in sets.iter().enumerate() { + let nal = &avcc[*start..*start + *len]; + let header = nal.first().copied().ok_or_else(|| Error::Invalid("empty avcC parameter set".into()))?; + let expected = if i < nsps { nal_type::SPS } else { nal_type::PPS }; + ensure!(header & 0x1f == expected, "avcC parameter set has NAL type {}, expected {expected}", header & 0x1f); + } + for (start, len) in sets { + self.handle_nal(&avcc[start..start + len], 0)?; + } + self.nal_length_size = Some((avcc[4] & 3) as usize + 1); + Ok(()) } /// Length of NAL length prefixes (from avcC), or None for Annex-B input. @@ -675,3 +686,17 @@ fn make_picture(o: &Output) -> Picture { draft: o.meta.draft, } } + +#[cfg(test)] +mod config_tests { + use super::Decoder; + + #[test] + fn avcc_rejects_picture_slices_without_starting_a_picture() { + let config = [1, 66, 0, 30, 0xff, 0xe1, 0, 2, 0x41, 0x80, 0]; + let mut decoder = Decoder::with_threads(1); + assert!(decoder.configure_avcc(&config).is_err()); + assert!(decoder.pending.is_none()); + assert_eq!(decoder.stats().pictures, 0); + } +} diff --git a/crates/h264/src/synth_tests.rs b/crates/h264/src/synth_tests.rs index a0c9da2f2..8588c5402 100644 --- a/crates/h264/src/synth_tests.rs +++ b/crates/h264/src/synth_tests.rs @@ -82,6 +82,20 @@ fn pps(cabac: bool, weighted: bool) -> Vec { nal(3, 8, &w.finish()) } +#[test] +fn avcc_accepts_generated_sps_and_pps() { + let sps = sps(66, 4, false, PocType::T2); + let pps = pps(false, false); + let (sps, pps) = (&sps[4..], &pps[4..]); + let mut avcc = vec![1, 66, 0, 30, 0xff, 0xe1]; + avcc.extend_from_slice(&(sps.len() as u16).to_be_bytes()); + avcc.extend_from_slice(sps); + avcc.push(1); + avcc.extend_from_slice(&(pps.len() as u16).to_be_bytes()); + avcc.extend_from_slice(pps); + assert!(Decoder::from_avcc(&avcc).is_ok()); +} + #[derive(Default, Clone)] struct Pic { idr: bool, diff --git a/crates/hevc/src/decoder.rs b/crates/hevc/src/decoder.rs index 4b46044da..a78600f20 100644 --- a/crates/hevc/src/decoder.rs +++ b/crates/hevc/src/decoder.rs @@ -160,11 +160,12 @@ impl Decoder { pub fn configure_hvcc(&mut self, hvcc: &[u8]) -> Result<()> { ensure!(hvcc.len() >= 23, "hvcC too short"); ensure!(hvcc[0] == 1, "unsupported hvcC version {}", hvcc[0]); - self.nal_length_size = Some((hvcc[21] & 3) as usize + 1); let num_arrays = hvcc[22] as usize; let mut pos = 23; + let mut sets = Vec::new(); for _ in 0..num_arrays { ensure!(pos + 3 <= hvcc.len(), "hvcC truncated"); + let array_type = hvcc[pos] & 0x3f; let n = u16::from_be_bytes([hvcc[pos + 1], hvcc[pos + 2]]) as usize; pos += 3; for _ in 0..n { @@ -172,10 +173,19 @@ impl Decoder { let len = u16::from_be_bytes([hvcc[pos], hvcc[pos + 1]]) as usize; pos += 2; ensure!(pos + len <= hvcc.len(), "hvcC truncated"); - self.handle_nal(&hvcc[pos..pos + len], 0)?; + sets.push((array_type, &hvcc[pos..pos + len])); pos += len; } } + for (array_type, nal) in &sets { + let header = NalHeader::parse(nal)?; + ensure!([nal_type::VPS, nal_type::SPS, nal_type::PPS].contains(array_type), "hvcC array has non-parameter-set NAL type {array_type}"); + ensure!(header.nal_type == *array_type, "hvcC array type {array_type} disagrees with NAL header type {}", header.nal_type); + } + for (_, nal) in sets { + self.handle_nal(nal, 0)?; + } + self.nal_length_size = Some((hvcc[21] & 3) as usize + 1); Ok(()) } @@ -651,3 +661,20 @@ fn make_picture(o: &Output) -> Picture { draft: o.meta.draft, } } + +#[cfg(test)] +mod config_tests { + use super::Decoder; + + #[test] + fn hvcc_rejects_picture_slices_without_starting_a_picture() { + let mut config = vec![0; 23]; + config[0] = 1; + config[22] = 1; + config.extend_from_slice(&[32, 0, 1, 0, 2, 0x02, 0x01]); + let mut decoder = Decoder::with_threads(1); + assert!(decoder.configure_hvcc(&config).is_err()); + assert!(decoder.pending.is_none()); + assert_eq!(decoder.stats().pictures, 0); + } +} diff --git a/crates/hevc/src/synth_tests.rs b/crates/hevc/src/synth_tests.rs index 4c2f1c968..52a6e177e 100644 --- a/crates/hevc/src/synth_tests.rs +++ b/crates/hevc/src/synth_tests.rs @@ -644,3 +644,18 @@ fn picture_store_is_checked_against_the_memory_budget() { assert!(!decodes_within(&stream, pictures * one - 1)); assert!(decodes_within(&stream, deckcraft_bitstream::budget::DECODE_MEMORY_BUDGET)); } + +#[test] +fn hvcc_accepts_generated_vps_sps_and_pps() { + let sets = [vps(), sps(true), pps(0, &None, false)]; + let mut hvcc = vec![0; 23]; + hvcc[0] = 1; + hvcc[22] = sets.len() as u8; + for (kind, set) in [32, 33, 34].into_iter().zip(sets) { + let nal = &set[4..]; + hvcc.extend_from_slice(&[kind, 0, 1]); + hvcc.extend_from_slice(&(nal.len() as u16).to_be_bytes()); + hvcc.extend_from_slice(nal); + } + assert!(Decoder::from_hvcc(&hvcc).is_ok()); +} From 2c11865b2da6caf8189652adad99cb8c87b600c5 Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Fri, 30 Oct 2026 16:49:21 -0500 Subject: [PATCH 2/5] codec: accept hvcC SEI arrays without allocation --- crates/h264/src/decoder.rs | 14 -------- crates/h264/src/synth_tests.rs | 34 +++++++++++++++++++ crates/hevc/src/decoder.rs | 61 ++++++++++++++++++---------------- crates/hevc/src/synth_tests.rs | 43 ++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 43 deletions(-) diff --git a/crates/h264/src/decoder.rs b/crates/h264/src/decoder.rs index b4b429b9a..a3244d473 100644 --- a/crates/h264/src/decoder.rs +++ b/crates/h264/src/decoder.rs @@ -687,17 +687,3 @@ fn make_picture(o: &Output) -> Picture { draft: o.meta.draft, } } - -#[cfg(test)] -mod config_tests { - use super::Decoder; - - #[test] - fn avcc_rejects_picture_slices_without_starting_a_picture() { - let config = [1, 66, 0, 30, 0xff, 0xe1, 0, 2, 0x41, 0x80, 0]; - let mut decoder = Decoder::with_threads(1); - assert!(decoder.configure_avcc(&config).is_err()); - assert!(decoder.pending.is_none()); - assert_eq!(decoder.stats().pictures, 0); - } -} diff --git a/crates/h264/src/synth_tests.rs b/crates/h264/src/synth_tests.rs index 8588c5402..24a104cf2 100644 --- a/crates/h264/src/synth_tests.rs +++ b/crates/h264/src/synth_tests.rs @@ -96,6 +96,40 @@ fn avcc_accepts_generated_sps_and_pps() { assert!(Decoder::from_avcc(&avcc).is_ok()); } +#[test] +fn avcc_rejects_a_valid_picture_slice_without_starting_a_picture() { + let sequence = sps(66, 4, false, PocType::T0); + let picture = pps(false, false); + let slice = slice_nal( + &Pic { + idr: true, + frame_num: 0, + ref_idc: 3, + long_term_ref: false, + mods: vec![], + mmcos: vec![], + mbs: vec![Some(vec![128; 16 * 16]); NMB], + b: false, + poc_lsb: Some(0), + poc_delta: None, + wt: None, + }, + false, + ); + let mut avcc = vec![1, 66, 0, 30, 0xff, 0xe2]; + for nal in [&sequence[4..], &slice[4..]] { + avcc.extend_from_slice(&(nal.len() as u16).to_be_bytes()); + avcc.extend_from_slice(nal); + } + avcc.push(1); + avcc.extend_from_slice(&((picture.len() - 4) as u16).to_be_bytes()); + avcc.extend_from_slice(&picture[4..]); + + let mut decoder = Decoder::with_threads(1); + assert!(decoder.configure_avcc(&avcc).is_err()); + assert_eq!(decoder.stats().pictures, 0); +} + #[derive(Default, Clone)] struct Pic { idr: bool, diff --git a/crates/hevc/src/decoder.rs b/crates/hevc/src/decoder.rs index d3419103f..b4ad29f98 100644 --- a/crates/hevc/src/decoder.rs +++ b/crates/hevc/src/decoder.rs @@ -161,30 +161,33 @@ impl Decoder { ensure!(hvcc.len() >= 23, "hvcC too short"); ensure!(hvcc[0] == 1, "unsupported hvcC version {}", hvcc[0]); let num_arrays = hvcc[22] as usize; - let mut pos = 23; - let mut sets = Vec::new(); - for _ in 0..num_arrays { - ensure!(pos + 3 <= hvcc.len(), "hvcC truncated"); - let array_type = hvcc[pos] & 0x3f; - let n = u16::from_be_bytes([hvcc[pos + 1], hvcc[pos + 2]]) as usize; - pos += 3; - for _ in 0..n { - ensure!(pos + 2 <= hvcc.len(), "hvcC truncated"); - let len = u16::from_be_bytes([hvcc[pos], hvcc[pos + 1]]) as usize; - pos += 2; - ensure!(pos + len <= hvcc.len(), "hvcC truncated"); - sets.push((array_type, &hvcc[pos..pos + len])); - pos += len; + for pass in 0..2 { + let mut pos = 23; + for _ in 0..num_arrays { + ensure!(pos <= hvcc.len() && hvcc.len() - pos >= 3, "hvcC truncated"); + let array_type = hvcc[pos] & 0x3f; + let n = u16::from_be_bytes([hvcc[pos + 1], hvcc[pos + 2]]) as usize; + pos += 3; + for _ in 0..n { + ensure!(pos <= hvcc.len() && hvcc.len() - pos >= 2, "hvcC truncated"); + let len = u16::from_be_bytes([hvcc[pos], hvcc[pos + 1]]) as usize; + pos += 2; + ensure!(len <= hvcc.len() - pos, "hvcC truncated"); + let nal = &hvcc[pos..pos + len]; + if pass == 0 { + let header = NalHeader::parse(nal)?; + ensure!( + [nal_type::VPS, nal_type::SPS, nal_type::PPS, nal_type::PREFIX_SEI, nal_type::SUFFIX_SEI].contains(&array_type), + "hvcC array has unsupported NAL type {array_type}" + ); + ensure!(header.nal_type == array_type, "hvcC array type {array_type} disagrees with NAL header type {}", header.nal_type); + } else { + self.handle_nal(nal, 0)?; + } + pos += len; + } } } - for (array_type, nal) in &sets { - let header = NalHeader::parse(nal)?; - ensure!([nal_type::VPS, nal_type::SPS, nal_type::PPS].contains(array_type), "hvcC array has non-parameter-set NAL type {array_type}"); - ensure!(header.nal_type == *array_type, "hvcC array type {array_type} disagrees with NAL header type {}", header.nal_type); - } - for (_, nal) in sets { - self.handle_nal(nal, 0)?; - } self.nal_length_size = Some((hvcc[21] & 3) as usize + 1); Ok(()) } @@ -669,14 +672,14 @@ mod config_tests { use super::Decoder; #[test] - fn hvcc_rejects_picture_slices_without_starting_a_picture() { + fn hvcc_skips_prefix_and_suffix_sei_arrays() { let mut config = vec![0; 23]; config[0] = 1; - config[22] = 1; - config.extend_from_slice(&[32, 0, 1, 0, 2, 0x02, 0x01]); - let mut decoder = Decoder::with_threads(1); - assert!(decoder.configure_hvcc(&config).is_err()); - assert!(decoder.pending.is_none()); - assert_eq!(decoder.stats().pictures, 0); + config[22] = 2; + config.extend_from_slice(&[39, 0, 1, 0, 2, 0x4e, 0x01]); + config.extend_from_slice(&[40, 0, 1, 0, 2, 0x50, 0x01]); + + let decoder = Decoder::from_hvcc(&config); + assert!(decoder.is_ok()); } } diff --git a/crates/hevc/src/synth_tests.rs b/crates/hevc/src/synth_tests.rs index 52a6e177e..7baf7b496 100644 --- a/crates/hevc/src/synth_tests.rs +++ b/crates/hevc/src/synth_tests.rs @@ -659,3 +659,46 @@ fn hvcc_accepts_generated_vps_sps_and_pps() { } assert!(Decoder::from_hvcc(&hvcc).is_ok()); } + +#[test] +fn hvcc_rejects_a_valid_picture_slice_without_starting_a_picture() { + let sets = [vps(), sps(true), pps(0, &None, false)]; + let mut stream = Vec::new(); + let mut saved = None; + encode_picture( + &PicDesc { + nal_type: 19, + poc: 0, + intra: true, + cu: Cu::Pcm, + pps_id: 0, + st: vec![], + lt: vec![], + num_ref: 0, + list_entry: None, + segments: whole(WC * HC), + }, + &None, + &content(0), + &mut stream, + &mut saved, + ); + + let mut hvcc = vec![0; 23]; + hvcc[0] = 1; + hvcc[22] = 4; + for (kind, set) in [32, 33, 34].into_iter().zip(sets) { + let nal = &set[4..]; + hvcc.extend_from_slice(&[kind, 0, 1]); + hvcc.extend_from_slice(&(nal.len() as u16).to_be_bytes()); + hvcc.extend_from_slice(nal); + } + let slice = &stream[4..]; + hvcc.extend_from_slice(&[19, 0, 1]); + hvcc.extend_from_slice(&(slice.len() as u16).to_be_bytes()); + hvcc.extend_from_slice(slice); + + let mut decoder = Decoder::with_threads(1); + assert!(decoder.configure_hvcc(&hvcc).is_err()); + assert_eq!(decoder.stats().pictures, 0); +} From ddd39249ba40256ce7370d08a3cd4e863d9c306b Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Fri, 30 Oct 2026 17:03:36 -0500 Subject: [PATCH 3/5] test(codec): strengthen configuration slice regressions --- crates/h264/src/synth_tests.rs | 8 +++++--- crates/hevc/src/synth_tests.rs | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/h264/src/synth_tests.rs b/crates/h264/src/synth_tests.rs index 24a104cf2..a2f9988ae 100644 --- a/crates/h264/src/synth_tests.rs +++ b/crates/h264/src/synth_tests.rs @@ -116,14 +116,16 @@ fn avcc_rejects_a_valid_picture_slice_without_starting_a_picture() { }, false, ); - let mut avcc = vec![1, 66, 0, 30, 0xff, 0xe2]; - for nal in [&sequence[4..], &slice[4..]] { + let mut avcc = vec![1, 66, 0, 30, 0xff, 0xe1]; + for nal in [&sequence[4..]] { avcc.extend_from_slice(&(nal.len() as u16).to_be_bytes()); avcc.extend_from_slice(nal); } - avcc.push(1); + avcc.push(2); avcc.extend_from_slice(&((picture.len() - 4) as u16).to_be_bytes()); avcc.extend_from_slice(&picture[4..]); + avcc.extend_from_slice(&((slice.len() - 4) as u16).to_be_bytes()); + avcc.extend_from_slice(&slice[4..]); let mut decoder = Decoder::with_threads(1); assert!(decoder.configure_avcc(&avcc).is_err()); diff --git a/crates/hevc/src/synth_tests.rs b/crates/hevc/src/synth_tests.rs index 7baf7b496..4d044a3d5 100644 --- a/crates/hevc/src/synth_tests.rs +++ b/crates/hevc/src/synth_tests.rs @@ -700,5 +700,6 @@ fn hvcc_rejects_a_valid_picture_slice_without_starting_a_picture() { let mut decoder = Decoder::with_threads(1); assert!(decoder.configure_hvcc(&hvcc).is_err()); + assert!(decoder.flush().is_empty()); assert_eq!(decoder.stats().pictures, 0); } From 7190c858604fb2c94d332554cdeeb8ad755d0707 Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Fri, 30 Oct 2026 17:06:36 -0500 Subject: [PATCH 4/5] test(h264): avoid singleton fixture loop --- crates/h264/src/synth_tests.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/crates/h264/src/synth_tests.rs b/crates/h264/src/synth_tests.rs index a2f9988ae..ac00d8604 100644 --- a/crates/h264/src/synth_tests.rs +++ b/crates/h264/src/synth_tests.rs @@ -117,10 +117,9 @@ fn avcc_rejects_a_valid_picture_slice_without_starting_a_picture() { false, ); let mut avcc = vec![1, 66, 0, 30, 0xff, 0xe1]; - for nal in [&sequence[4..]] { - avcc.extend_from_slice(&(nal.len() as u16).to_be_bytes()); - avcc.extend_from_slice(nal); - } + let nal = &sequence[4..]; + avcc.extend_from_slice(&(nal.len() as u16).to_be_bytes()); + avcc.extend_from_slice(nal); avcc.push(2); avcc.extend_from_slice(&((picture.len() - 4) as u16).to_be_bytes()); avcc.extend_from_slice(&picture[4..]); From 3c068df62879ce6444d0d04562d3b483def6a85e Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Fri, 30 Oct 2026 17:13:38 -0500 Subject: [PATCH 5/5] test(h264): complete picture slice fixture --- crates/h264/src/synth_tests.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/h264/src/synth_tests.rs b/crates/h264/src/synth_tests.rs index ac00d8604..76609dbde 100644 --- a/crates/h264/src/synth_tests.rs +++ b/crates/h264/src/synth_tests.rs @@ -108,7 +108,7 @@ fn avcc_rejects_a_valid_picture_slice_without_starting_a_picture() { long_term_ref: false, mods: vec![], mmcos: vec![], - mbs: vec![Some(vec![128; 16 * 16]); NMB], + mbs: vec![Some(vec![128; 384]); NMB], b: false, poc_lsb: Some(0), poc_delta: None, @@ -128,6 +128,7 @@ fn avcc_rejects_a_valid_picture_slice_without_starting_a_picture() { let mut decoder = Decoder::with_threads(1); assert!(decoder.configure_avcc(&avcc).is_err()); + assert!(decoder.flush().is_empty()); assert_eq!(decoder.stats().pictures, 0); }