On 3/13/2020 7:28 AM, Anton Khirnov wrote:
> This tells the parsing functions the payload size and prevents them from
> overreading.
> ---
> libavcodec/h264_sei.c | 30 +++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
> index a565feabe2..a2452141ca 100644
> --- a/libavcodec/h264_sei.c
> +++ b/libavcodec/h264_sei.c
> @@ -407,9 +407,9 @@ int ff_h264_sei_decode(H264SEIContext *h, GetBitContext
> *gb,
> int master_ret = 0;
>
> while (get_bits_left(gb) > 16 && show_bits(gb, 16)) {
> + GetBitContext gb_payload;
> int type = 0;
> unsigned size = 0;
> - unsigned next;
> int ret = 0;
>
> do {
> @@ -429,35 +429,38 @@ int ff_h264_sei_decode(H264SEIContext *h, GetBitContext
> *gb,
> type, 8*size, get_bits_left(gb));
> return AVERROR_INVALIDDATA;
> }
> - next = get_bits_count(gb) + 8 * size;
> +
> + ret = init_get_bits8(&gb_payload, gb->buffer + get_bits_count(gb) /
> 8, size);
> + if (ret < 0)
> + return ret;
>
> switch (type) {
> case H264_SEI_TYPE_PIC_TIMING: // Picture timing SEI
> - ret = decode_picture_timing(&h->picture_timing, gb, ps, logctx);
> + ret = decode_picture_timing(&h->picture_timing, &gb_payload, ps,
> logctx);
> break;
> case H264_SEI_TYPE_USER_DATA_REGISTERED:
> - ret = decode_registered_user_data(h, gb, logctx, size);
> + ret = decode_registered_user_data(h, &gb_payload, logctx, size);
> break;
> case H264_SEI_TYPE_USER_DATA_UNREGISTERED:
> - ret = decode_unregistered_user_data(&h->unregistered, gb,
> logctx, size);
> + ret = decode_unregistered_user_data(&h->unregistered,
> &gb_payload, logctx, size);
> break;
> case H264_SEI_TYPE_RECOVERY_POINT:
> - ret = decode_recovery_point(&h->recovery_point, gb, logctx);
> + ret = decode_recovery_point(&h->recovery_point, &gb_payload,
> logctx);
> break;
> case H264_SEI_TYPE_BUFFERING_PERIOD:
> - ret = decode_buffering_period(&h->buffering_period, gb, ps,
> logctx);
> + ret = decode_buffering_period(&h->buffering_period, &gb_payload,
> ps, logctx);
> break;
> case H264_SEI_TYPE_FRAME_PACKING:
> - ret = decode_frame_packing_arrangement(&h->frame_packing, gb);
> + ret = decode_frame_packing_arrangement(&h->frame_packing,
> &gb_payload);
> break;
> case H264_SEI_TYPE_DISPLAY_ORIENTATION:
> - ret = decode_display_orientation(&h->display_orientation, gb);
> + ret = decode_display_orientation(&h->display_orientation,
> &gb_payload);
> break;
> case H264_SEI_TYPE_GREEN_METADATA:
> - ret = decode_green_metadata(&h->green_metadata, gb);
> + ret = decode_green_metadata(&h->green_metadata, &gb_payload);
> break;
> case H264_SEI_TYPE_ALTERNATIVE_TRANSFER:
> - ret = decode_alternative_transfer(&h->alternative_transfer, gb);
> + ret = decode_alternative_transfer(&h->alternative_transfer,
> &gb_payload);
> break;
> default:
> av_log(logctx, AV_LOG_DEBUG, "unknown SEI type %d\n", type);
> @@ -467,10 +470,7 @@ int ff_h264_sei_decode(H264SEIContext *h, GetBitContext
> *gb,
> if (ret < 0)
> master_ret = ret;
>
> - skip_bits_long(gb, next - get_bits_count(gb));
> -
> - // FIXME check bits here
> - align_get_bits(gb);
> + skip_bits_long(gb, 8 * size);
> }
>
> return master_ret;
LGTM.
May be worth adding an EXPLODE err_recognition check and a warning/error
message (depending on if we abort or not) in case get_bits_left < 0. I
noticed this is the case for the sample in fate-h264-attachment-631,
where a seemingly incorrect SPS in extradata makes the buffering period
SEI function read more bits than available.
_______________________________________________
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".
