[FFmpeg-devel] [PATCH 14/17] avformat/avc: Be more strict when parsing SPS
Andreas Rheinhardt
andreas.rheinhardt at gmail.com
Thu Jul 9 22:20:19 EEST 2020
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.rheinhardt at 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
More information about the ffmpeg-devel
mailing list