[FFmpeg-devel] [PATCH] avcodec/argo: Check for end of input in decode_alcd()

[Andreas Rheinhardt]

Andreas Rheinhardt:
> Michael Niedermayer:
>> Fixes: reading over the end
>> Fixes: 36346/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ARGO_fuzzer-5366943107383296
>>
>> 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/argo.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/libavcodec/argo.c b/libavcodec/argo.c
>> index bbdb6ae15f..79a44d2583 100644
>> --- a/libavcodec/argo.c
>> +++ b/libavcodec/argo.c
>> @@ -116,6 +116,8 @@ static int decode_alcd(AVCodecContext *avctx, AVFrame *frame)
>>              int index;
>>  
>>              if (count == 0) {
>> +                if (bytestream2_get_bytes_left(gb) < 1)
>> +                    return AVERROR_INVALIDDATA;
>>                  codes = bytestream2_get_byteu(&sb);
>>                  count = 8;
>>              }
>>
> Does the following also fix the issue?
> 
> diff --git a/libavcodec/argo.c b/libavcodec/argo.c
> 
> index bbdb6ae15f..602c042568 100644
> 
> --- a/libavcodec/argo.c
> 
> +++ b/libavcodec/argo.c
> 
> @@ -102,13 +102,14 @@ static int decode_alcd(AVCodecContext *avctx,
> AVFrame *frame)
> 
>      uint8_t *dst = frame->data[0];
> 
>      uint8_t codes = 0;
> 
>      int count = 0;
> 
> +    int num_codes = ((frame->width + 1) / 2 * (frame->height + 1) / 2 +
> 7) >> 3;
> 
> 
> 
> -    if (bytestream2_get_bytes_left(gb) < 1024 + (((frame->width / 2) *
> (frame->height / 2) + 7) >> 3))
> 
> +    if (bytestream2_get_bytes_left(gb) < 1024 + num_codes)
> 
>          return AVERROR_INVALIDDATA;
> 
> 
> 
>      bytestream2_skipu(gb, 1024);
> 
>      sb = *gb;
> 
> -    bytestream2_skipu(gb, ((frame->width / 2) * (frame->height / 2) +
> 7) >> 3);
> 
> +    bytestream2_skipu(gb, num_codes);
> 
> 
> 
>      for (int y = 0; y < frame->height; y += 2) {
> 
>          for (int x = 0; x < frame->width; x += 2) {
> 
As can be seen from the above patch, my guess is that odd dimensions are
the cause (because num_codes as above is the number of codes that is
actually read); but my patch would not only change the criterion for
when to error out, but also how much to skip (i.e. where the real data
begins) and this makes me wonder whether we should not error out in this
case (and ask for a sample).

- Andreas