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

[Michael Niedermayer]

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.

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. 

thx


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200531/9a83357b/attachment.sig>