This commit adds some basic checks for invalid values when parsing an SPS; e.g. the earlier code did not check whether the values read from the bitstream get truncated when returned via the H264SPS struct (whose members are uint8_t), so that a caller could not even check for this error itself.
Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com> --- libavformat/avc.c | 91 ++++++++++++++++++++++++++++---------------- libavformat/avc.h | 4 +- libavformat/mxfenc.c | 2 +- 3 files changed, 61 insertions(+), 36 deletions(-) diff --git a/libavformat/avc.c b/libavformat/avc.c index 9f375ca992..6246410ad0 100644 --- a/libavformat/avc.c +++ b/libavformat/avc.c @@ -27,19 +27,32 @@ #include "avc.h" #include "avio_internal.h" -static inline unsigned get_ue_golomb(GetBitContext *gb) +static int get_ue_golomb(GetBitContext *gb, unsigned *val) { int i; for (i = 1; i <= 32; i++) { if (get_bits_left(gb) < i) - return 0; + return AVERROR_INVALIDDATA; if (show_bits1(gb)) break; skip_bits1(gb); } if (i > 32) - return 0; - return get_bits_long(gb, i) - 1; + return AVERROR_INVALIDDATA; + *val = get_bits_long(gb, i) - 1; + return 0; +} + +static int get_se_golomb(GetBitContext *gb, int *val) +{ + unsigned v; + int sign, ret = get_ue_golomb(gb, &v); + + if (ret < 0) + return ret; + sign = -(v & 1); + *val = ((v >> 1) ^ sign) - sign; + return 0; } static const uint8_t *avc_find_startcode_internal(const uint8_t *p, const uint8_t *end) @@ -242,8 +255,8 @@ int ff_isom_write_avcc(AVIOContext *pb, const uint8_t *data, int len) goto fail; avio_w8(pb, 0xfc | seq.chroma_format_idc); /* 6 bits reserved (111111) + chroma_format_idc */ - avio_w8(pb, 0xf8 | (seq.bit_depth_luma - 8)); /* 5 bits reserved (11111) + bit_depth_luma_minus8 */ - avio_w8(pb, 0xf8 | (seq.bit_depth_chroma - 8)); /* 5 bits reserved (11111) + bit_depth_chroma_minus8 */ + avio_w8(pb, 0xf8 | seq.bit_depth_luma_minus8); /* 5 bits reserved (11111) + bit_depth_luma_minus8 */ + avio_w8(pb, 0xf8 | seq.bit_depth_chroma_minus8); /* 5 bits reserved (11111) + bit_depth_chroma_minus8 */ avio_w8(pb, nb_sps_ext); /* number of sps ext */ if (nb_sps_ext) avio_write(pb, sps_ext, sps_ext_size); @@ -357,12 +370,26 @@ static const AVRational avc_sample_aspect_ratio[17] = { { 2, 1 }, }; -static inline int get_se_golomb(GetBitContext *gb) { - unsigned v = get_ue_golomb(gb) + 1; - int sign = -(v & 1); - return ((v >> 1) ^ sign) - sign; -} - +#define GET_UE_GOLOMB(dst, max) do { \ + unsigned val; \ + ret = get_ue_golomb(&gb, &val); \ + if (ret < 0) \ + goto end; \ + if (val > (max)) { \ + ret = AVERROR_INVALIDDATA; \ + goto end; \ + } \ + (dst) = val; \ + } while (0) +#define GET_SE_GOLOMB(dst) do { \ + int val; \ + ret = get_se_golomb(&gb, &val); \ + if (ret < 0) \ + goto end; \ + (dst) = val; \ + } while (0) +#define SKIP_UE_GOLOMB GET_UE_GOLOMB(val, UINT_MAX) +#define SKIP_SE_GOLOMB GET_SE_GOLOMB(val) int ff_avc_decode_sps(H264SPS *sps, const uint8_t *buf, int buf_size) { int i, j, ret, rbsp_size, aspect_ratio_idc, pic_order_cnt_type; @@ -391,19 +418,19 @@ int ff_avc_decode_sps(H264SPS *sps, const uint8_t *buf, int buf_size) sps->constraint_set_flags |= get_bits1(&gb) << 5; // constraint_set5_flag skip_bits(&gb, 2); // reserved_zero_2bits sps->level_idc = get_bits(&gb, 8); - sps->id = get_ue_golomb(&gb); + GET_UE_GOLOMB(sps->id, H264_MAX_SPS_COUNT - 1); if (sps->profile_idc == 100 || sps->profile_idc == 110 || sps->profile_idc == 122 || sps->profile_idc == 244 || sps->profile_idc == 44 || sps->profile_idc == 83 || sps->profile_idc == 86 || sps->profile_idc == 118 || sps->profile_idc == 128 || sps->profile_idc == 138 || sps->profile_idc == 139 || sps->profile_idc == 134) { - sps->chroma_format_idc = get_ue_golomb(&gb); // chroma_format_idc + GET_UE_GOLOMB(sps->chroma_format_idc, 3); if (sps->chroma_format_idc == 3) { skip_bits1(&gb); // separate_colour_plane_flag } - sps->bit_depth_luma = get_ue_golomb(&gb) + 8; - sps->bit_depth_chroma = get_ue_golomb(&gb) + 8; + GET_UE_GOLOMB(sps->bit_depth_luma_minus8, 6); + GET_UE_GOLOMB(sps->bit_depth_chroma_minus8, 6); skip_bits1(&gb); // qpprime_y_zero_transform_bypass_flag if (get_bits1(&gb)) { // seq_scaling_matrix_present_flag for (i = 0; i < ((sps->chroma_format_idc != 3) ? 8 : 12); i++) { @@ -414,7 +441,7 @@ int ff_avc_decode_sps(H264SPS *sps, const uint8_t *buf, int buf_size) sizeOfScalingList = i < 6 ? 16 : 64; for (j = 0; j < sizeOfScalingList; j++) { if (nextScale != 0) { - delta_scale = get_se_golomb(&gb); + GET_SE_GOLOMB(delta_scale); nextScale = (lastScale + delta_scale) & 0xff; } lastScale = nextScale == 0 ? lastScale : nextScale; @@ -423,28 +450,26 @@ int ff_avc_decode_sps(H264SPS *sps, const uint8_t *buf, int buf_size) } } else { sps->chroma_format_idc = 1; - sps->bit_depth_luma = 8; - sps->bit_depth_chroma = 8; } - get_ue_golomb(&gb); // log2_max_frame_num_minus4 - pic_order_cnt_type = get_ue_golomb(&gb); + SKIP_UE_GOLOMB; // log2_max_frame_num_minus4 + GET_UE_GOLOMB(pic_order_cnt_type, 2); if (pic_order_cnt_type == 0) { - get_ue_golomb(&gb); // log2_max_pic_order_cnt_lsb_minus4 + SKIP_UE_GOLOMB; // log2_max_pic_order_cnt_lsb_minus4 } else if (pic_order_cnt_type == 1) { skip_bits1(&gb); // delta_pic_order_always_zero - get_se_golomb(&gb); // offset_for_non_ref_pic - get_se_golomb(&gb); // offset_for_top_to_bottom_field - num_ref_frames_in_pic_order_cnt_cycle = get_ue_golomb(&gb); + SKIP_SE_GOLOMB; // offset_for_non_ref_pic + SKIP_SE_GOLOMB; // offset_for_top_to_bottom_field + GET_UE_GOLOMB(num_ref_frames_in_pic_order_cnt_cycle, 255); for (i = 0; i < num_ref_frames_in_pic_order_cnt_cycle; i++) - get_se_golomb(&gb); // offset_for_ref_frame + SKIP_SE_GOLOMB; // offset_for_ref_frame } - get_ue_golomb(&gb); // max_num_ref_frames + SKIP_UE_GOLOMB; // max_num_ref_frames skip_bits1(&gb); // gaps_in_frame_num_value_allowed_flag - get_ue_golomb(&gb); // pic_width_in_mbs_minus1 - get_ue_golomb(&gb); // pic_height_in_map_units_minus1 + SKIP_UE_GOLOMB; // pic_width_in_mbs_minus1 + SKIP_UE_GOLOMB; // pic_height_in_map_units_minus1 sps->frame_mbs_only_flag = get_bits1(&gb); if (!sps->frame_mbs_only_flag) @@ -453,10 +478,10 @@ int ff_avc_decode_sps(H264SPS *sps, const uint8_t *buf, int buf_size) skip_bits1(&gb); // direct_8x8_inference_flag if (get_bits1(&gb)) { // frame_cropping_flag - get_ue_golomb(&gb); // frame_crop_left_offset - get_ue_golomb(&gb); // frame_crop_right_offset - get_ue_golomb(&gb); // frame_crop_top_offset - get_ue_golomb(&gb); // frame_crop_bottom_offset + SKIP_UE_GOLOMB; // frame_crop_left_offset + SKIP_UE_GOLOMB; // frame_crop_right_offset + SKIP_UE_GOLOMB; // frame_crop_top_offset + SKIP_UE_GOLOMB; // frame_crop_bottom_offset } if (get_bits1(&gb)) { // vui_parameters_present_flag diff --git a/libavformat/avc.h b/libavformat/avc.h index b3df0a7b6b..db1d3334dd 100644 --- a/libavformat/avc.h +++ b/libavformat/avc.h @@ -44,8 +44,8 @@ typedef struct { uint8_t level_idc; uint8_t constraint_set_flags; uint8_t chroma_format_idc; - uint8_t bit_depth_luma; - uint8_t bit_depth_chroma; + uint8_t bit_depth_luma_minus8; + uint8_t bit_depth_chroma_minus8; uint8_t frame_mbs_only_flag; AVRational sar; } H264SPS; diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c index 5a3a609bf6..5eeeb9ab84 100644 --- a/libavformat/mxfenc.c +++ b/libavformat/mxfenc.c @@ -2206,7 +2206,7 @@ static int mxf_parse_h264_frame(AVFormatContext *s, AVStream *st, sc->aspect_ratio.num, sc->aspect_ratio.den, 1024*1024); intra_only = (sps->constraint_set_flags >> 3) & 1; sc->interlaced = !sps->frame_mbs_only_flag; - sc->component_depth = sps->bit_depth_luma; + sc->component_depth = sps->bit_depth_luma_minus8 + 8; buf = nal_end; break; -- 2.20.1 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".