diff --git a/crates/bitstream/src/lib.rs b/crates/bitstream/src/lib.rs index b5acf036..48884673 100644 --- a/crates/bitstream/src/lib.rs +++ b/crates/bitstream/src/lib.rs @@ -134,6 +134,12 @@ impl<'a> BitReader<'a> { u32::try_from(r).map_err(|_| BitError::InvalidExpGolomb) } + /// `ue(v) + add` (the `..._minus1` / `..._minus8` fields), failing instead of overflowing on a + /// hostile value near `u32::MAX`. + pub fn read_ue_plus(&mut self, add: u32) -> Result { + self.read_ue()?.checked_add(add).ok_or(BitError::InvalidExpGolomb) + } + /// Signed Exp-Golomb `se(v)`. pub fn read_se(&mut self) -> Result { let k = self.read_ue()? as i64; @@ -343,6 +349,18 @@ mod tests { use super::*; use proptest::prelude::*; + #[test] + fn read_ue_plus_rejects_overflow() { + let mut w = BitWriter::new(); + w.write_ue(7); + w.write_ue(u32::MAX); + w.rbsp_trailing(); + let b = w.finish(); + let mut r = BitReader::new(&b); + assert_eq!(r.read_ue_plus(1).unwrap(), 8); + assert!(r.read_ue_plus(1).is_err()); + } + #[test] fn reads_bits() { let d = [0b1010_1100, 0b0101_0011]; diff --git a/crates/h264/src/params.rs b/crates/h264/src/params.rs index f6a3a9e7..a7e35d73 100644 --- a/crates/h264/src/params.rs +++ b/crates/h264/src/params.rs @@ -150,7 +150,7 @@ pub struct HrdParameters { } fn parse_hrd(r: &mut BitReader) -> Result { - let cpb_cnt = r.read_ue()? + 1; + let cpb_cnt = r.read_ue_plus(1)?; ensure!(cpb_cnt <= 32, "cpb_cnt_minus1 out of range"); let mut h = HrdParameters { cpb_cnt, bit_rate_scale: r.read_bits(4)? as u8, cpb_size_scale: r.read_bits(4)? as u8, ..Default::default() }; for _ in 0..cpb_cnt { @@ -353,8 +353,8 @@ impl Sps { if chroma_format_idc == 3 { separate_colour_plane = r.read_flag()?; } - bit_depth_luma = r.read_ue()? + 8; - bit_depth_chroma = r.read_ue()? + 8; + bit_depth_luma = r.read_ue_plus(8)?; + bit_depth_chroma = r.read_ue_plus(8)?; ensure!(bit_depth_luma <= 14 && bit_depth_chroma <= 14, "bit depth out of range"); bypass = r.read_flag()?; seq_scaling_matrix_present = r.read_flag()?; @@ -363,7 +363,7 @@ impl Sps { scaling_lists = parse_scaling_lists(&mut r, count, None)?; } } - let log2_max_frame_num = r.read_ue()? + 4; + let log2_max_frame_num = r.read_ue_plus(4)?; ensure!(log2_max_frame_num <= 16, "log2_max_frame_num out of range"); let pic_order_cnt_type = r.read_ue()?; ensure!(pic_order_cnt_type <= 2, "pic_order_cnt_type out of range"); @@ -373,7 +373,7 @@ impl Sps { let mut offset_for_top_to_bottom_field = 0; let mut offset_for_ref_frame = Vec::new(); if pic_order_cnt_type == 0 { - log2_max_poc_lsb = r.read_ue()? + 4; + log2_max_poc_lsb = r.read_ue_plus(4)?; ensure!(log2_max_poc_lsb <= 16, "log2_max_pic_order_cnt_lsb out of range"); } else if pic_order_cnt_type == 1 { delta_pic_order_always_zero = r.read_flag()?; @@ -388,8 +388,8 @@ impl Sps { let max_num_ref_frames = r.read_ue()?; ensure!(max_num_ref_frames <= 16, "max_num_ref_frames out of range"); let gaps_in_frame_num_allowed = r.read_flag()?; - let pic_width_in_mbs = r.read_ue()? + 1; - let pic_height_in_map_units = r.read_ue()? + 1; + let pic_width_in_mbs = r.read_ue_plus(1)?; + let pic_height_in_map_units = r.read_ue_plus(1)?; ensure!(pic_width_in_mbs <= 1024 && pic_height_in_map_units <= 1024, "picture too large"); let frame_mbs_only = r.read_flag()?; let mb_adaptive_frame_field = if !frame_mbs_only { r.read_flag()? } else { false }; @@ -429,7 +429,8 @@ impl Sps { }; if let Some((l, rr, t, b)) = sps.frame_crop { let (cx, cy) = sps.crop_unit(); - ensure!((l + rr) * cx < sps.width() && (t + b) * cy < sps.height(), "frame cropping exceeds picture size"); + let crop = |a: u32, b: u32, unit: u32| (u64::from(a) + u64::from(b)) * u64::from(unit); + ensure!(crop(l, rr, cx) < u64::from(sps.width()) && crop(t, b, cy) < u64::from(sps.height()), "frame cropping exceeds picture size"); } Ok(sps) } @@ -572,7 +573,7 @@ impl Pps { .ok_or_else(|| crate::Error::MissingParameterSet(format!("SPS {sps_id} for PPS {id}")))?; let entropy_coding_mode = r.read_flag()?; let bottom_field_pic_order_in_frame_present = r.read_flag()?; - let num_slice_groups = r.read_ue()? + 1; + let num_slice_groups = r.read_ue_plus(1)?; ensure!(num_slice_groups <= 8, "num_slice_groups out of range"); let mut slice_group_map_type = 0; if num_slice_groups > 1 { @@ -595,7 +596,7 @@ impl Pps { r.read_ue()?; } 6 => { - let n = r.read_ue()? + 1; + let n = r.read_ue_plus(1)?; let bits = 32 - (num_slice_groups - 1).leading_zeros(); for _ in 0..n { r.read_bits(bits)?; @@ -604,8 +605,8 @@ impl Pps { _ => {} } } - let num_ref_idx_l0_default_active = r.read_ue()? + 1; - let num_ref_idx_l1_default_active = r.read_ue()? + 1; + let num_ref_idx_l0_default_active = r.read_ue_plus(1)?; + let num_ref_idx_l1_default_active = r.read_ue_plus(1)?; ensure!(num_ref_idx_l0_default_active <= 32 && num_ref_idx_l1_default_active <= 32, "num_ref_idx_default_active out of range"); let weighted_pred = r.read_flag()?; let weighted_bipred_idc = r.read_bits(2)?; @@ -716,4 +717,38 @@ mod tests { assert_eq!(sps.pic_order_cnt_type, 2); assert_eq!(sps.crop_rect(), (0, 0, 176, 144)); } + + /// The baseline SPS of `parse_minimal_baseline_sps` with a chosen width and optional cropping. + fn sps_with(width_minus1: u32, crop: Option<[u32; 4]>) -> Vec { + let mut w = BitWriter::new(); + w.write_bits(66, 8); + w.write_bits(0xC0, 8); + w.write_bits(30, 8); + w.write_ue(0); + w.write_ue(0); + w.write_ue(2); + w.write_ue(1); + w.write_bit(false); + w.write_ue(width_minus1); + w.write_ue(8); + w.write_bit(true); // frame_mbs_only + w.write_bit(true); // direct_8x8 + w.write_bit(crop.is_some()); + if let Some(c) = crop { + c.iter().for_each(|&v| w.write_ue(v)); + } + w.write_bit(false); // vui + w.rbsp_trailing(); + w.finish() + } + + #[test] + fn hostile_sps_values_are_errors_not_overflows() { + // pic_width_in_mbs_minus1 = u32::MAX: `+ 1` used to overflow (wrapping to 0 in release). + assert!(Sps::parse(&sps_with(u32::MAX, None)).is_err()); + // Crop offsets whose sum or product exceed u32. + assert!(Sps::parse(&sps_with(10, Some([u32::MAX - 1, u32::MAX - 1, 0, 0]))).is_err()); + assert!(Sps::parse(&sps_with(10, Some([0x8000_0000, 0, 0, 0]))).is_err()); + assert!(Sps::parse(&sps_with(10, Some([1, 1, 1, 1]))).is_ok()); + } } diff --git a/crates/h264/src/slice.rs b/crates/h264/src/slice.rs index c80cf0e0..f53edbfd 100644 --- a/crates/h264/src/slice.rs +++ b/crates/h264/src/slice.rs @@ -199,9 +199,9 @@ impl SliceHeader { let mut num_ref_idx_active = [pps.num_ref_idx_l0_default_active, pps.num_ref_idx_l1_default_active]; if !slice_type.is_intra() { if r.read_flag()? { - num_ref_idx_active[0] = r.read_ue()? + 1; + num_ref_idx_active[0] = r.read_ue_plus(1)?; if slice_type.is_b() { - num_ref_idx_active[1] = r.read_ue()? + 1; + num_ref_idx_active[1] = r.read_ue_plus(1)?; } } let max = if field_pic { 32 } else { 16 }; diff --git a/crates/hevc/src/params.rs b/crates/hevc/src/params.rs index fbcff559..0044a6ac 100644 --- a/crates/hevc/src/params.rs +++ b/crates/hevc/src/params.rs @@ -97,7 +97,7 @@ pub fn skip_hrd(r: &mut BitReader, common: bool, max_sub_layers_minus1: u32) -> } let mut cpb_cnt = 1; if !low_delay { - cpb_cnt = r.read_ue()? + 1; + cpb_cnt = r.read_ue_plus(1)?; ensure!(cpb_cnt <= 32, "cpb_cnt_minus1 out of range"); } if nal { @@ -259,7 +259,7 @@ impl ScalingList { while m < 6 { let coef_num = if size == 0 { 16 } else { 64 }; if !r.read_flag()? { - let delta = r.read_ue()? as usize * step; + let delta = (r.read_ue()? as usize).saturating_mul(step); ensure!(delta <= m, "scaling_list_pred_matrix_id_delta out of range"); if delta == 0 { // default list @@ -349,12 +349,13 @@ impl StRps { pub fn parse(r: &mut BitReader, idx: usize, sets: &[StRps], num_sets: usize) -> Result { let inter = if idx != 0 { r.read_flag()? } else { false }; if inter { - let delta_idx = if idx == num_sets { r.read_ue()? as usize + 1 } else { 1 }; + let delta_idx = if idx == num_sets { r.read_ue_plus(1)? as usize } else { 1 }; ensure!(delta_idx <= idx, "delta_idx_minus1 out of range"); let rf = &sets[idx - delta_idx]; let sign = r.read_flag()?; - let abs = r.read_ue()? as i32 + 1; + let abs = r.read_ue_plus(1)?; ensure!(abs <= 1 << 15, "abs_delta_rps_minus1 out of range"); + let abs = abs as i32; let delta_rps = if sign { -abs } else { abs }; let n = rf.num_delta_pocs(); let mut used = vec![false; n + 1]; @@ -406,15 +407,17 @@ impl StRps { let mut out = StRps::default(); let mut poc = 0i32; for _ in 0..nneg { - let d = r.read_ue()? as i32 + 1; + let d = r.read_ue_plus(1)?; ensure!(d <= 1 << 15, "delta_poc_s0_minus1 out of range"); + let d = d as i32; poc -= d; out.s0.push((poc, r.read_flag()?)); } poc = 0; for _ in 0..npos { - let d = r.read_ue()? as i32 + 1; + let d = r.read_ue_plus(1)?; ensure!(d <= 1 << 15, "delta_poc_s1_minus1 out of range"); + let d = d as i32; poc += d; out.s1.push((poc, r.read_flag()?)); } @@ -491,25 +494,25 @@ impl Sps { if r.read_flag()? { conf_win = (r.read_ue()?, r.read_ue()?, r.read_ue()?, r.read_ue()?); } - let bit_depth_luma = r.read_ue()? + 8; - let bit_depth_chroma = r.read_ue()? + 8; + let bit_depth_luma = r.read_ue_plus(8)?; + let bit_depth_chroma = r.read_ue_plus(8)?; ensure!(bit_depth_luma <= 16 && bit_depth_chroma <= 16, "bit depth out of range"); - let log2_max_poc_lsb = r.read_ue()? + 4; + let log2_max_poc_lsb = r.read_ue_plus(4)?; ensure!(log2_max_poc_lsb <= 16, "log2_max_pic_order_cnt_lsb_minus4 out of range"); let ordering_all = r.read_flag()?; let (mut dpb, mut reorder, mut latency) = (1, 0, 0); let start = if ordering_all { 0 } else { max_sub_layers_minus1 }; for _ in start..=max_sub_layers_minus1 { // the values of the highest sub-layer are used - dpb = r.read_ue()? + 1; + dpb = r.read_ue_plus(1)?; reorder = r.read_ue()?; latency = r.read_ue()?; } ensure!(dpb <= 16 && reorder <= 16, "sps_max_dec_pic_buffering out of range"); - let log2_min_cb = r.read_ue()? + 3; - let log2_ctb = log2_min_cb + r.read_ue()?; - let log2_min_tb = r.read_ue()? + 2; - let log2_max_tb = log2_min_tb + r.read_ue()?; + let log2_min_cb = r.read_ue_plus(3)?; + let log2_ctb = log2_min_cb.saturating_add(r.read_ue()?); + let log2_min_tb = r.read_ue_plus(2)?; + let log2_max_tb = log2_min_tb.saturating_add(r.read_ue()?); ensure!((4..=6).contains(&log2_ctb) && log2_min_cb <= log2_ctb, "CTB size out of range"); ensure!(log2_min_tb < log2_min_cb && log2_max_tb <= log2_ctb.min(5), "transform block sizes out of range"); ensure!(width % (1 << log2_min_cb) == 0 && height % (1 << log2_min_cb) == 0, "picture size not a multiple of MinCbSizeY"); @@ -532,8 +535,8 @@ impl Sps { if pcm { pcm_bit_depth_luma = r.read_bits(4)? + 1; pcm_bit_depth_chroma = r.read_bits(4)? + 1; - log2_min_pcm = r.read_ue()? + 3; - log2_max_pcm = log2_min_pcm + r.read_ue()?; + log2_min_pcm = r.read_ue_plus(3)?; + log2_max_pcm = log2_min_pcm.saturating_add(r.read_ue()?); pcm_loop_filter_disabled = r.read_flag()?; ensure!(pcm_bit_depth_luma <= bit_depth_luma && pcm_bit_depth_chroma <= bit_depth_chroma, "PCM bit depth out of range"); ensure!(log2_max_pcm <= log2_ctb.min(5), "PCM size out of range"); @@ -637,10 +640,10 @@ impl Sps { pub fn crop_rect(&self) -> (u32, u32, u32, u32) { let (l, r, t, b) = self.conf_win; let (sw, sh) = (self.sub_width_c(), self.sub_height_c()); - let x = (l * sw).min(self.width - 1); - let y = (t * sh).min(self.height - 1); - let w = self.width.saturating_sub(x + r * sw).max(1); - let h = self.height.saturating_sub(y + b * sh).max(1); + let x = l.saturating_mul(sw).min(self.width - 1); + let y = t.saturating_mul(sh).min(self.height - 1); + let w = self.width.saturating_sub(x.saturating_add(r.saturating_mul(sw))).max(1); + let h = self.height.saturating_sub(y.saturating_add(b.saturating_mul(sh))).max(1); (x, y, w, h) } @@ -720,8 +723,8 @@ impl Pps { let num_extra_slice_header_bits = r.read_bits(3)?; let sign_data_hiding = r.read_flag()?; let cabac_init_present = r.read_flag()?; - let num_ref_idx_l0_default = r.read_ue()? + 1; - let num_ref_idx_l1_default = r.read_ue()? + 1; + let num_ref_idx_l0_default = r.read_ue_plus(1)?; + let num_ref_idx_l1_default = r.read_ue_plus(1)?; ensure!(num_ref_idx_l0_default <= 15 && num_ref_idx_l1_default <= 15, "num_ref_idx_default_active out of range"); let init_qp = 26 + r.read_se()?; let constrained_intra_pred = r.read_flag()?; @@ -742,8 +745,8 @@ impl Pps { let (mut column_widths, mut row_heights) = (Vec::new(), Vec::new()); let mut loop_filter_across_tiles = true; if tiles_enabled { - num_tile_columns = r.read_ue()? + 1; - num_tile_rows = r.read_ue()? + 1; + num_tile_columns = r.read_ue_plus(1)?; + num_tile_rows = r.read_ue_plus(1)?; ensure!(num_tile_columns <= 64 && num_tile_rows <= 64, "too many tiles"); uniform_spacing = r.read_flag()?; if !uniform_spacing { @@ -770,7 +773,7 @@ impl Pps { } let scaling_list = if r.read_flag()? { Some(ScalingList::parse(&mut r)?) } else { None }; let lists_modification_present = r.read_flag()?; - let log2_parallel_merge_level = r.read_ue()? + 2; + let log2_parallel_merge_level = r.read_ue_plus(2)?; ensure!(log2_parallel_merge_level <= 6, "log2_parallel_merge_level out of range"); let slice_header_extension_present = r.read_flag()?; let mut range_extension = false; @@ -933,3 +936,67 @@ impl Layout { self.tile_id[self.rs_to_ts[rs as usize] as usize] } } + +#[cfg(test)] +mod tests { + use super::*; + use deckcraft_bitstream::BitWriter; + + /// A 64x64 Main-profile SPS (CTB 64, no VUI) with a chosen conformance window and CTB-size delta. + fn sps_bits(conf_win: Option<[u32; 4]>, log2_diff_cb: u32) -> Vec { + let mut w = BitWriter::new(); + w.write_bits(0, 4); // vps id + w.write_bits(0, 3); // max_sub_layers_minus1 + w.write_bit(true); // temporal_id_nesting + w.write_bits(0, 2); // profile_space + w.write_bit(false); // tier + w.write_bits(1, 5); // profile_idc + w.write_bits(0x6000_0000, 32); // compatibility + w.write_bits(0b1001, 4); // progressive, interlaced, non_packed, frame_only + w.write_bits(0, 32); + w.write_bits(0, 12); // 43 constraint bits + 1 reserved + w.write_bits(93, 8); // level_idc + [0, 1, 64, 64].iter().for_each(|&v| w.write_ue(v)); // sps id, chroma 4:2:0, width, height + w.write_bit(conf_win.is_some()); + conf_win.iter().flatten().for_each(|&v| w.write_ue(v)); + [0, 0, 4].iter().for_each(|&v| w.write_ue(v)); // bit depths, log2_max_poc_lsb_minus4 + w.write_bit(true); // sub_layer_ordering_info_present + [0, 0, 0].iter().for_each(|&v| w.write_ue(v)); // dpb, reorder, latency + w.write_ue(0); // log2_min_cb_minus3 + w.write_ue(log2_diff_cb); + w.write_ue(0); // log2_min_tb_minus2 + w.write_ue(3); // log2_diff_max_min_tb + w.write_ue(1); // max_transform_hierarchy_depth_inter + w.write_ue(1); // ... intra + [false; 4].iter().for_each(|&b| w.write_bit(b)); // scaling list, amp, sao, pcm + w.write_ue(0); // num_short_term_ref_pic_sets + [false; 5].iter().for_each(|&b| w.write_bit(b)); // long term, temporal mvp, strong intra, vui, extension + w.rbsp_trailing(); + w.finish() + } + + #[test] + fn hostile_sps_values_are_errors_not_overflows() { + assert!(Sps::parse(&sps_bits(None, 3)).is_ok()); + // log2_diff_max_min_luma_coding_block_size = u32::MAX: `log2_min_cb + ue` used to overflow. + assert!(Sps::parse(&sps_bits(None, u32::MAX)).is_err()); + // Conformance window offsets whose product with SubWidthC / SubHeightC exceeds u32. + let sps = Sps::parse(&sps_bits(Some([u32::MAX; 4]), 3)).unwrap(); + let (x, y, w, h) = sps.crop_rect(); + assert!(x < 64 && y < 64 && w >= 1 && h >= 1, "{:?}", (x, y, w, h)); + } + + #[test] + fn hostile_st_rps_deltas_are_errors_not_overflows() { + for ue in [0x7FFF_FFFF, 0x8000_0000, u32::MAX] { + let mut w = BitWriter::new(); + w.write_ue(1); // num_negative_pics + w.write_ue(0); // num_positive_pics + w.write_ue(ue); // delta_poc_s0_minus1 + w.write_bit(true); + w.rbsp_trailing(); + let b = w.finish(); + assert!(StRps::parse(&mut BitReader::new(&b), 0, &[], 1).is_err(), "ue {ue:#x}"); + } + } +} diff --git a/crates/hevc/src/slice.rs b/crates/hevc/src/slice.rs index 6b4c7ae6..c7bd1e39 100644 --- a/crates/hevc/src/slice.rs +++ b/crates/hevc/src/slice.rs @@ -293,9 +293,9 @@ impl SliceHeader { if sh.slice_type != SliceType::I { sh.num_ref_idx = [pps.num_ref_idx_l0_default, if sh.is_b() { pps.num_ref_idx_l1_default } else { 0 }]; if r.read_flag()? { - sh.num_ref_idx[0] = r.read_ue()? + 1; + sh.num_ref_idx[0] = r.read_ue_plus(1)?; if sh.is_b() { - sh.num_ref_idx[1] = r.read_ue()? + 1; + sh.num_ref_idx[1] = r.read_ue_plus(1)?; } } ensure!(sh.num_ref_idx[0] <= 15 && sh.num_ref_idx[1] <= 15, "num_ref_idx_active out of range"); @@ -365,7 +365,7 @@ impl SliceHeader { let n = r.read_ue()?; ensure!(n <= sps.pic_width_in_ctbs() * sps.pic_height_in_ctbs(), "num_entry_point_offsets out of range"); if n > 0 { - let len = r.read_ue()? + 1; + let len = r.read_ue_plus(1)?; ensure!(len <= 32, "offset_len_minus1 out of range"); for _ in 0..n { sh.entry_points.push(r.read_bits(len)? + 1);