[FFmpeg-devel] [PATCH 1/4] avcodec/jpeg2000dec: clear pointer which become stale in get_ppt()

[Gautam Ramakrishnan]

On Sun, May 31, 2020 at 10:12 PM Michael Niedermayer
<michael at niedermayer.cc> wrote:
>
> On Sun, May 31, 2020 at 09:22:46PM +0530, Gautam Ramakrishnan wrote:
> > On Sun, May 31, 2020 at 7:21 PM Michael Niedermayer
> > <michael at niedermayer.cc> wrote:
> > >
> > > Fixes: use after free
> > > Fixes: 22484/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_JPEG2000_fuzzer-5671488765296640
> > >
> > > 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/jpeg2000dec.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
> > > index 65555424ed..b7766459c4 100644
> > > --- a/libavcodec/jpeg2000dec.c
> > > +++ b/libavcodec/jpeg2000dec.c
> > > @@ -928,6 +928,7 @@ static int get_ppt(Jpeg2000DecoderContext *s, int n)
> > >          tile->packed_headers = new;
> > >      } else
> > >          return AVERROR(ENOMEM);
> > > +    memset(&tile->packed_headers_stream, 0, sizeof(tile->packed_headers_stream));
> > >      memcpy(tile->packed_headers + tile->packed_headers_size,
> > >             s->g.buffer, n - 3);
> > >      tile->packed_headers_size += n - 3;
> > > --
> > > 2.17.1
> > >
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > > To unsubscribe, visit link above, or email
> > > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> >
> > I do not understand this. Wouldn't the memcpy overwrite the address anyway?
>
> The "GetByteContext      packed_headers_stream" can point to the stream prior
> to av_realloc(), and after realloc its no longer valid
> that needs to be cleared because if not, its possible this will be used and
> crash later.
Yep, got it.
>
> Note, this is not a valid jpeg2000 file, but some trash from the fuzzer.
> Its certainly possible to add a check elsewhere to avoid this.
> the memset was just the obvious solution that came to my mind.
>
> Also on this subject its quite possible that the jpeg2000 code is missing
> more such saftey checks
> Iam mentioning this as you are working on jpeg2000. its always a good
> idea to keep the question "can this be made to do something nasty from crafted input"
> in mind when writing code.
I shall keep this in mind.
>
> thx
>
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> The educated differ from the uneducated as much as the living from the
> dead. -- Aristotle
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".



-- 
-------------
Gautam |