[FFmpeg-devel] [PATCH] avcodec/vp9_superframe_split_bsf: Check in size

[Andreas Rheinhardt]

Michael Niedermayer:
> On Sat, Mar 19, 2022 at 06:38:08PM +0100, Michael Niedermayer wrote:
>> On Fri, Mar 18, 2022 at 06:56:19PM +0100, Andreas Rheinhardt wrote:
>>> Michael Niedermayer:
>>>> Fixes: Out of array read
>>>> Fixes: 45137/clusterfuzz-testcase-minimized-ffmpeg_BSF_VP9_SUPERFRAME_SPLIT_fuzzer-4984270639202304
>>>>
>>>> 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/vp9_superframe_split_bsf.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavcodec/vp9_superframe_split_bsf.c b/libavcodec/vp9_superframe_split_bsf.c
>>>> index ed0444561a..481484a4f0 100644
>>>> --- a/libavcodec/vp9_superframe_split_bsf.c
>>>> +++ b/libavcodec/vp9_superframe_split_bsf.c
>>>> @@ -51,7 +51,7 @@ static int vp9_superframe_split_filter(AVBSFContext *ctx, AVPacket *out)
>>>>              return ret;
>>>>          in = s->buffer_pkt;
>>>>  
>>>> -        marker = in->data[in->size - 1];
>>>> +        marker = in->size ? in->data[in->size - 1] : 0;
>>>>          if ((marker & 0xe0) == 0xc0) {
>>>>              int length_size = 1 + ((marker >> 3) & 0x3);
>>>>              int   nb_frames = 1 + (marker & 0x7);
>>>
>>> There is a second place in this BSF where data might be read in the
>>> absence of data, namely if one of the frames in a superframe have size
>>> of zero (its attempted to read its profile; no actual read takes place
>>> due to the checks of the get_bits API, but it is nevertheless invalid
>>> data). See
>>> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200530160541.29517-7-andreas.rheinhardt@gmail.com/;
> 
> The get bits API checks for NULL data, if data is not NULL it must be padded
> even when size is 0.
> Nothing against the 2nd check, but thats a seperate issue

I know that there is no invalid read (and said as much)

> 
> 
>>> also see
>>> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200530160541.29517-11-andreas.rheinhardt@gmail.com/
> 
> please apply your bugfixes! especially if its about out or array accesses
> 

Will do.

> 
>>> and
>>> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20200530160541.29517-1-andreas.rheinhardt@gmail.com/
> 
> thats now found by the fuzzer too, in 45722
> if you dont apply your fix i will post a fix
> 
> thx
> 
> [...]
> 
>