[FFmpeg-devel] [PATCH v2] avcodec/interplayvideo: Check sizes of decode buffers

[Paul B Mahol]

On 7/3/17, Hein-Pieter van Braam <hp at tmm.cx> wrote:
> On Mon, 2017-07-03 at 11:43 +0200, Paul B Mahol wrote:
>> On 7/2/17, Hein-Pieter van Braam <hp at tmm.cx> wrote:
>> > Fixes: 6503 crash with fuzzed file
>> >
>> > Signed-off-by: Hein-Pieter van Braam <hp at tmm.cx>
>> > ---
>> >  libavcodec/interplayvideo.c | 8 ++++++++
>> >  1 file changed, 8 insertions(+)
>> >
>> > diff --git a/libavcodec/interplayvideo.c
>> > b/libavcodec/interplayvideo.c
>> > index d6f484a..86530e6 100644
>> > --- a/libavcodec/interplayvideo.c
>> > +++ b/libavcodec/interplayvideo.c
>> > @@ -972,6 +972,8 @@ static void
>> > ipvideo_decode_format_06_opcodes(IpvideoContext *s, AVFrame *frame)
>> >                          x, y, opcode, bytestream2_tell(&s-
>> > >stream_ptr));
>> >
>> >                  s->pixel_ptr = frame->data[0] + x + y * frame-
>> > >linesize[0];
>> > +                if (s->pixel_ptr > (s->pixel_ptr +
>> > s->upper_motion_limit_offset))
>> > +                    return;
>>
>> This looks strange.
>>
>
> The code already has a feature to ensure that movement vectors can't
> write past the end of the target AVFrame. I thought I'd reuse this to
> prevent writing past the end of the current AVFrame for the 'regular'
> pixeldata too. This code checks the value of the pointer into the
> AVFrame's pixeldata is not further than the last 8x8 block so that we
> can't write past it.
>
> Would there be a better way of doing this?
>
> Although, I suppose if I check the size of the AVFrame itself instead
> then it would also be safe because of the for loop. I can also
> implement it like that if that's preferred?

You are comparing pointer with same pointer increased by some value.

This is wrong way to fix it.