[FFmpeg-devel] [PATCH 4/4] avcodec/h264_slice: Fix overflow in recovery_frame computation

[James Almer]

On 6/8/2018 7:11 PM, Michael Niedermayer wrote:
> Fixes: signed integer overflow: 15 + 2147483646 cannot be represented in type 'int'
> Fixes: 8381/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_H264_fuzzer-6225533137321984
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> ---
>  libavcodec/h264_sei.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
> index 9defcb80b9..2f16d95f56 100644
> --- a/libavcodec/h264_sei.c
> +++ b/libavcodec/h264_sei.c
> @@ -261,10 +261,16 @@ static int decode_unregistered_user_data(H264SEIUnregistered *h, GetBitContext *
>      return 0;
>  }
>  
> -static int decode_recovery_point(H264SEIRecoveryPoint *h, GetBitContext *gb)
> +static int decode_recovery_point(H264SEIRecoveryPoint *h, GetBitContext *gb, void *logctx)
>  {
> -    h->recovery_frame_cnt = get_ue_golomb_long(gb);
> +    unsigned recovery_frame_cnt = get_ue_golomb_long(gb);
>  
> +    if (recovery_frame_cnt > (1<<16)) {

Maybe move MAX_LOG2_MAX_FRAME_NUM out of h264_ps.c and into h264_ps.h,
then use it here?

> +        av_log(logctx, AV_LOG_ERROR, "recovery_frame_cnt %d is out of range\n", recovery_frame_cnt);

It's unsigned, so %u. Some pedantic compilers (Like djgpp) may complain
or downright fail otherwise.

> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    h->recovery_frame_cnt = recovery_frame_cnt;
>      /* 1b exact_match_flag,
>       * 1b broken_link_flag,
>       * 2b changing_slice_group_idc */
> @@ -429,7 +435,7 @@ int ff_h264_sei_decode(H264SEIContext *h, GetBitContext *gb,
>              ret = decode_unregistered_user_data(&h->unregistered, gb, logctx, size);
>              break;
>          case H264_SEI_TYPE_RECOVERY_POINT:
> -            ret = decode_recovery_point(&h->recovery_point, gb);
> +            ret = decode_recovery_point(&h->recovery_point, gb, logctx);
>              break;
>          case H264_SEI_TYPE_BUFFERING_PERIOD:
>              ret = decode_buffering_period(&h->buffering_period, gb, ps, logctx);
>