From 8be04f459b3b298167d0cb0ad90b7eeda7e1b016 Mon Sep 17 00:00:00 2001 From: qwu16 Date: Mon, 4 Sep 2023 10:15:57 +0800 Subject: [PATCH] Fix fuzzing issue reported by Chromium fuzzing test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: chromium:1475195, chromium:1475944, chromium:1475909 Change-Id: Iaa9dc6570a8b70ec58efe0a64d468e1cae4cb484 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/317504 Reviewed-by: Sergey Silkin Reviewed-by: Erik Språng Commit-Queue: Sergey Silkin Cr-Commit-Position: refs/heads/main@{#40696} --- common_video/h265/h265_bitstream_parser.cc | 29 ++++++------- .../h265/h265_bitstream_parser_unittest.cc | 35 ++++++++++++++++ common_video/h265/h265_pps_parser.cc | 6 --- common_video/h265/h265_sps_parser.cc | 42 ++++++++----------- common_video/h265/h265_sps_parser.h | 8 ++-- 5 files changed, 70 insertions(+), 50 deletions(-) diff --git a/common_video/h265/h265_bitstream_parser.cc b/common_video/h265/h265_bitstream_parser.cc index 250939815c..ee77166705 100644 --- a/common_video/h265/h265_bitstream_parser.cc +++ b/common_video/h265/h265_bitstream_parser.cc @@ -151,8 +151,6 @@ H265BitstreamParser::Result H265BitstreamParser::ParseNonParameterSetNalu( } uint32_t num_long_term_sps = 0; uint32_t num_long_term_pics = 0; - std::vector lt_idx_sps; - std::vector poc_lsb_lt; std::vector used_by_curr_pic_lt_flag; bool short_term_ref_pic_set_sps_flag = false; uint32_t short_term_ref_pic_set_idx = 0; @@ -188,6 +186,8 @@ H265BitstreamParser::Result H265BitstreamParser::ParseNonParameterSetNalu( if (short_term_ref_pic_set_idx_bits > 0) { short_term_ref_pic_set_idx = slice_reader.ReadBits(short_term_ref_pic_set_idx_bits); + IN_RANGE_OR_RETURN(short_term_ref_pic_set_idx, 0, + sps->num_short_term_ref_pic_sets - 1); } } if (sps->long_term_ref_pics_present_flag) { @@ -201,27 +201,26 @@ H265BitstreamParser::Result H265BitstreamParser::ParseNonParameterSetNalu( num_long_term_pics = slice_reader.ReadExponentialGolomb(); IN_RANGE_OR_RETURN(num_long_term_pics, 0, kMaxLongTermRefPicSets - num_long_term_sps); - lt_idx_sps.resize(num_long_term_sps + num_long_term_pics, 0); used_by_curr_pic_lt_flag.resize(num_long_term_sps + num_long_term_pics, 0); - poc_lsb_lt.resize(num_long_term_sps + num_long_term_pics, 0); for (uint32_t i = 0; i < num_long_term_sps + num_long_term_pics; i++) { if (i < num_long_term_sps) { - uint32_t lt_idx_sps_bits = 0; + uint32_t lt_idx_sps = 0; if (sps->num_long_term_ref_pics_sps > 1) { // lt_idx_sps: u(v) - lt_idx_sps_bits = + uint32_t lt_idx_sps_bits = H265::Log2Ceiling(sps->num_long_term_ref_pics_sps); - lt_idx_sps[i] = slice_reader.ReadBits(lt_idx_sps_bits); + lt_idx_sps = slice_reader.ReadBits(lt_idx_sps_bits); + IN_RANGE_OR_RETURN(lt_idx_sps, 0, + sps->num_long_term_ref_pics_sps - 1); } - poc_lsb_lt[i] = sps->lt_ref_pic_poc_lsb_sps[lt_idx_sps_bits]; used_by_curr_pic_lt_flag[i] = - sps->used_by_curr_pic_lt_sps_flag[lt_idx_sps_bits]; + sps->used_by_curr_pic_lt_sps_flag[lt_idx_sps]; } else { // poc_lsb_lt: u(v) uint32_t poc_lsb_lt_bits = sps->log2_max_pic_order_cnt_lsb_minus4 + 4; - poc_lsb_lt[i] = slice_reader.ReadBits(poc_lsb_lt_bits); + slice_reader.ConsumeBits(poc_lsb_lt_bits); // used_by_curr_pic_lt_flag: u(1) used_by_curr_pic_lt_flag[i] = slice_reader.Read(); } @@ -375,20 +374,22 @@ H265BitstreamParser::Result H265BitstreamParser::ParseNonParameterSetNalu( return kUnsupportedStream; } // five_minus_max_num_merge_cand: ue(v) - int five_minus_max_num_merge_cand = slice_reader.ReadExponentialGolomb(); + uint32_t five_minus_max_num_merge_cand = + slice_reader.ReadExponentialGolomb(); IN_RANGE_OR_RETURN(5 - five_minus_max_num_merge_cand, 1, 5); } } // slice_qp_delta: se(v) int32_t last_slice_qp_delta = slice_reader.ReadSignedExponentialGolomb(); - IN_RANGE_OR_RETURN(26 + pps->init_qp_minus26 + last_slice_qp_delta, - -pps->qp_bd_offset_y, 51); if (!slice_reader.Ok() || (abs(last_slice_qp_delta) > kMaxAbsQpDeltaValue)) { // Something has gone wrong, and the parsed value is invalid. - RTC_LOG(LS_WARNING) << "Parsed QP value out of range."; + RTC_LOG(LS_ERROR) << "Parsed QP value out of range."; return kInvalidStream; } + // 7-54 in H265 spec. + IN_RANGE_OR_RETURN(26 + pps->init_qp_minus26 + last_slice_qp_delta, + -pps->qp_bd_offset_y, 51); last_slice_qp_delta_ = last_slice_qp_delta; last_slice_pps_id_ = pps_id; diff --git a/common_video/h265/h265_bitstream_parser_unittest.cc b/common_video/h265/h265_bitstream_parser_unittest.cc index 34986e667a..7ca979433a 100644 --- a/common_video/h265/h265_bitstream_parser_unittest.cc +++ b/common_video/h265/h265_bitstream_parser_unittest.cc @@ -67,6 +67,30 @@ const uint8_t kH265SliceStrChunk[] = { 0xbe, 0x6b, 0x15, 0x48, 0x59, 0x1f, 0xf7, 0xc1, 0x7c, 0xe2, 0xe8, 0x10, }; +// Contains enough of the image slice to contain invalid slice QP -52. +const uint8_t kH265BitstreamInvalidQPChunk[] = { + 0x00, 0x00, 0x00, 0x01, 0x40, 0x01, 0x0c, 0x01, 0xff, 0xff, 0x04, 0x08, + 0x00, 0x00, 0x03, 0x00, 0x9d, 0x08, 0x00, 0x00, 0x03, 0x00, 0x00, 0x78, + 0x95, 0x98, 0x09, 0x00, 0x00, 0x00, 0x01, 0x42, 0x01, 0x01, 0x04, 0x08, + 0x00, 0x00, 0x03, 0x00, 0x9d, 0x08, 0x00, 0x00, 0x03, 0x00, 0x00, 0x78, + 0xb0, 0x03, 0xc0, 0x80, 0x10, 0xe5, 0x96, 0x56, 0x69, 0x24, 0xca, 0xe0, + 0x10, 0x00, 0x00, 0x03, 0x00, 0x10, 0x00, 0x00, 0x03, 0x01, 0xe0, 0x80, + 0x00, 0x00, 0x00, 0x01, 0x44, 0x01, 0xc1, 0x72, 0xb4, 0x62, 0x40, 0x00, + 0x00, 0x01, 0x26, 0x01, 0xaf, 0x03, 0x4c, +}; + +// Contains enough of the image slice to contain invalid slice QP 52. +const uint8_t kH265BitstreamInvalidQPChunk52[] = { + 0x00, 0x00, 0x00, 0x01, 0x40, 0x01, 0x0c, 0x01, 0xff, 0xff, 0x04, 0x08, + 0x00, 0x00, 0x03, 0x00, 0x9d, 0x08, 0x00, 0x00, 0x03, 0x00, 0x00, 0x78, + 0x95, 0x98, 0x09, 0x00, 0x00, 0x00, 0x01, 0x42, 0x01, 0x01, 0x04, 0x08, + 0x00, 0x00, 0x03, 0x00, 0x9d, 0x08, 0x00, 0x00, 0x03, 0x00, 0x00, 0x78, + 0xb0, 0x03, 0xc0, 0x80, 0x10, 0xe5, 0x96, 0x56, 0x69, 0x24, 0xca, 0xe0, + 0x10, 0x00, 0x00, 0x03, 0x00, 0x10, 0x00, 0x00, 0x03, 0x01, 0xe0, 0x80, + 0x00, 0x00, 0x00, 0x01, 0x44, 0x01, 0xc1, 0x72, 0xb4, 0x62, 0x40, 0x00, + 0x00, 0x01, 0x26, 0x01, 0xaf, 0x03, 0x44, +}; + TEST(H265BitstreamParserTest, ReportsNoQpWithoutParsedSlices) { H265BitstreamParser h265_parser; EXPECT_FALSE(h265_parser.GetLastSliceQp().has_value()); @@ -109,4 +133,15 @@ TEST(H265BitstreamParserTest, PpsIdFromSlice) { EXPECT_EQ(1u, *pps_id); } +TEST(H265BitstreamParserTest, ReportsLastSliceQpInvalidQPSlices) { + H265BitstreamParser h265_parser; + h265_parser.ParseBitstream(kH265BitstreamInvalidQPChunk); + absl::optional qp = h265_parser.GetLastSliceQp(); + ASSERT_FALSE(qp.has_value()); + + h265_parser.ParseBitstream(kH265BitstreamInvalidQPChunk52); + qp = h265_parser.GetLastSliceQp(); + ASSERT_FALSE(qp.has_value()); +} + } // namespace webrtc diff --git a/common_video/h265/h265_pps_parser.cc b/common_video/h265/h265_pps_parser.cc index 419fe312ba..1cc9abd794 100644 --- a/common_video/h265/h265_pps_parser.cc +++ b/common_video/h265/h265_pps_parser.cc @@ -228,12 +228,6 @@ absl::optional H265PpsParser::ParseInternal( } // lists_modification_present_flag: u(1) pps.lists_modification_present_flag = reader.Read(); - // log2_parallel_merge_level_minus2: ue(v) - uint32_t log2_parallel_merge_level_minus2 = reader.ReadExponentialGolomb(); - IN_RANGE_OR_RETURN_NULL(log2_parallel_merge_level_minus2, 0, - sps->ctb_log2_size_y - 2); - // slice_segment_header_extension_present_flag: u(1) - reader.ConsumeBits(1); if (!reader.Ok()) { return absl::nullopt; diff --git a/common_video/h265/h265_sps_parser.cc b/common_video/h265/h265_sps_parser.cc index 4dcfa1fcd9..96aee7c569 100644 --- a/common_video/h265/h265_sps_parser.cc +++ b/common_video/h265/h265_sps_parser.cc @@ -63,8 +63,6 @@ constexpr int kMaxNumCoefs = 64; namespace webrtc { -H265SpsParser::SpsState::SpsState() = default; - H265SpsParser::ShortTermRefPicSet::ShortTermRefPicSet() = default; H265SpsParser::ProfileTierLevel::ProfileTierLevel() = default; @@ -113,30 +111,23 @@ absl::optional H265SpsParser::ParseSps( } bool H265SpsParser::ParseScalingListData(BitstreamReader& reader) { - uint32_t scaling_list_pred_mode_flag[kMaxNumSizeIds][kMaxNumMatrixIds]; - int scaling_list_pred_matrix_id_delta[kMaxNumSizeIds][kMaxNumMatrixIds]; int32_t scaling_list_dc_coef_minus8[kMaxNumSizeIds][kMaxNumMatrixIds]; - int32_t scaling_list[kMaxNumSizeIds][kMaxNumMatrixIds][kMaxNumCoefs]; for (int size_id = 0; size_id < kMaxNumSizeIds; size_id++) { for (int matrix_id = 0; matrix_id < kMaxNumMatrixIds; matrix_id += (size_id == 3) ? 3 : 1) { // scaling_list_pred_mode_flag: u(1) - scaling_list_pred_mode_flag[size_id][matrix_id] = reader.Read(); - if (!scaling_list_pred_mode_flag[size_id][matrix_id]) { + bool scaling_list_pred_mode_flag = reader.Read(); + if (!scaling_list_pred_mode_flag) { // scaling_list_pred_matrix_id_delta: ue(v) - scaling_list_pred_matrix_id_delta[size_id][matrix_id] = - reader.ReadExponentialGolomb(); + int scaling_list_pred_matrix_id_delta = reader.ReadExponentialGolomb(); if (size_id <= 2) { - IN_RANGE_OR_RETURN_FALSE( - scaling_list_pred_matrix_id_delta[size_id][matrix_id], 0, - matrix_id); + IN_RANGE_OR_RETURN_FALSE(scaling_list_pred_matrix_id_delta, 0, + matrix_id); } else { // size_id == 3 - IN_RANGE_OR_RETURN_FALSE( - scaling_list_pred_matrix_id_delta[size_id][matrix_id], 0, - matrix_id / 3); + IN_RANGE_OR_RETURN_FALSE(scaling_list_pred_matrix_id_delta, 0, + matrix_id / 3); } } else { - int32_t next_coef = 8; uint32_t coef_num = std::min(kMaxNumCoefs, 1 << (4 + (size_id << 1))); if (size_id > 1) { // scaling_list_dc_coef_minus8: se(v) @@ -144,15 +135,12 @@ bool H265SpsParser::ParseScalingListData(BitstreamReader& reader) { reader.ReadSignedExponentialGolomb(); IN_RANGE_OR_RETURN_FALSE( scaling_list_dc_coef_minus8[size_id - 2][matrix_id], -7, 247); - next_coef = scaling_list_dc_coef_minus8[size_id - 2][matrix_id] + 8; } for (uint32_t i = 0; i < coef_num; i++) { // scaling_list_delta_coef: se(v) int32_t scaling_list_delta_coef = reader.ReadSignedExponentialGolomb(); IN_RANGE_OR_RETURN_FALSE(scaling_list_delta_coef, -128, 127); - next_coef = (next_coef + scaling_list_delta_coef + 256) % 256; - scaling_list[size_id][matrix_id][i] = next_coef; } } } @@ -192,7 +180,7 @@ H265SpsParser::ParseShortTermRefPicSet( uint32_t ref_rps_idx = st_rps_idx - (delta_idx_minus1 + 1); uint32_t num_delta_pocs = short_term_ref_pic_set[ref_rps_idx].num_delta_pocs; - RTC_CHECK_LT(num_delta_pocs, kMaxShortTermRefPicSets); + IN_RANGE_OR_RETURN_NULL(num_delta_pocs, 0, kMaxShortTermRefPicSets); const ShortTermRefPicSet& ref_set = short_term_ref_pic_set[ref_rps_idx]; bool used_by_curr_pic_flag[kMaxShortTermRefPicSets]; bool use_delta_flag[kMaxShortTermRefPicSets]; @@ -212,8 +200,9 @@ H265SpsParser::ParseShortTermRefPicSet( // and num_positive_pics. // Equation 7-61 int i = 0; - RTC_CHECK_LE(ref_set.num_negative_pics + ref_set.num_positive_pics, - kMaxShortTermRefPicSets); + IN_RANGE_OR_RETURN_NULL( + ref_set.num_negative_pics + ref_set.num_positive_pics, 0, + kMaxShortTermRefPicSets); for (int j = ref_set.num_positive_pics - 1; j >= 0; --j) { int d_poc = ref_set.delta_poc_s1[j] + delta_rps; if (d_poc < 0 && use_delta_flag[ref_set.num_negative_pics + j]) { @@ -258,6 +247,11 @@ H265SpsParser::ParseShortTermRefPicSet( } } st_ref_pic_set.num_positive_pics = i; + IN_RANGE_OR_RETURN_NULL(st_ref_pic_set.num_negative_pics, 0, + sps_max_dec_pic_buffering_minus1); + IN_RANGE_OR_RETURN_NULL( + st_ref_pic_set.num_positive_pics, 0, + sps_max_dec_pic_buffering_minus1 - st_ref_pic_set.num_negative_pics); } else { // num_negative_pics: ue(v) @@ -643,14 +637,12 @@ absl::optional H265SpsParser::ParseSpsInternal( sps.num_long_term_ref_pics_sps = reader.ReadExponentialGolomb(); IN_RANGE_OR_RETURN_NULL(sps.num_long_term_ref_pics_sps, 0, kMaxLongTermRefPicSets); - sps.lt_ref_pic_poc_lsb_sps.resize(sps.num_long_term_ref_pics_sps, 0); sps.used_by_curr_pic_lt_sps_flag.resize(sps.num_long_term_ref_pics_sps, 0); for (uint32_t i = 0; i < sps.num_long_term_ref_pics_sps; i++) { // lt_ref_pic_poc_lsb_sps: u(v) uint32_t lt_ref_pic_poc_lsb_sps_bits = sps.log2_max_pic_order_cnt_lsb_minus4 + 4; - sps.lt_ref_pic_poc_lsb_sps[i] = - reader.ReadBits(lt_ref_pic_poc_lsb_sps_bits); + reader.ConsumeBits(lt_ref_pic_poc_lsb_sps_bits); // used_by_curr_pic_lt_sps_flag: u(1) sps.used_by_curr_pic_lt_sps_flag[i] = reader.Read(); } diff --git a/common_video/h265/h265_sps_parser.h b/common_video/h265/h265_sps_parser.h index c3ac04c364..2dece2b722 100644 --- a/common_video/h265/h265_sps_parser.h +++ b/common_video/h265/h265_sps_parser.h @@ -75,15 +75,15 @@ class H265SpsParser { // The parsed state of the SPS. Only some select values are stored. // Add more as they are actually needed. struct SpsState { - SpsState(); + SpsState() = default; - uint32_t sps_max_sub_layers_minus1; + uint32_t sps_max_sub_layers_minus1 = 0; uint32_t chroma_format_idc = 0; uint32_t separate_colour_plane_flag = 0; uint32_t pic_width_in_luma_samples = 0; uint32_t pic_height_in_luma_samples = 0; uint32_t log2_max_pic_order_cnt_lsb_minus4 = 0; - uint32_t sps_max_dec_pic_buffering_minus1[kMaxSubLayers]; + uint32_t sps_max_dec_pic_buffering_minus1[kMaxSubLayers] = {}; uint32_t log2_min_luma_coding_block_size_minus3 = 0; uint32_t log2_diff_max_min_luma_coding_block_size = 0; uint32_t sample_adaptive_offset_enabled_flag = 0; @@ -91,7 +91,6 @@ class H265SpsParser { std::vector short_term_ref_pic_set; uint32_t long_term_ref_pics_present_flag = 0; uint32_t num_long_term_ref_pics_sps = 0; - std::vector lt_ref_pic_poc_lsb_sps; std::vector used_by_curr_pic_lt_sps_flag; uint32_t sps_temporal_mvp_enabled_flag = 0; uint32_t width = 0; @@ -100,7 +99,6 @@ class H265SpsParser { uint32_t vps_id = 0; uint32_t pic_width_in_ctbs_y = 0; uint32_t pic_height_in_ctbs_y = 0; - uint32_t ctb_log2_size_y = 0; uint32_t bit_depth_luma_minus8 = 0; };