| Message ID | 20181128004650.30626-1-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, Nov 28, 2018 at 1:54 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > > Fixes: Timeout > Fixes: 11318/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MSMPEG4V1_fuzzer-5710884555456512 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/msmpeg4dec.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libavcodec/msmpeg4dec.c b/libavcodec/msmpeg4dec.c > index 457a37e745..d278540ec2 100644 > --- a/libavcodec/msmpeg4dec.c > +++ b/libavcodec/msmpeg4dec.c > @@ -412,6 +412,9 @@ int ff_msmpeg4_decode_picture_header(MpegEncContext * s) > { > int code; > > + if (get_bits_left(&s->gb) * 8LL < (s->width+15)/16 * ((s->height+15)/16)) > + return AVERROR_INVALIDDATA; > + Please add a comment so such lines why these magic values where choosen, and an explanation in the commit message that explains the proof that these are an absolute limit and no valid frame could ever be smaller would be appreciated. - Hendrik
On Wed, Nov 28, 2018 at 10:06:12AM +0100, Hendrik Leppkes wrote: > On Wed, Nov 28, 2018 at 1:54 AM Michael Niedermayer > <michael@niedermayer.cc> wrote: > > > > Fixes: Timeout > > Fixes: 11318/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MSMPEG4V1_fuzzer-5710884555456512 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/msmpeg4dec.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/libavcodec/msmpeg4dec.c b/libavcodec/msmpeg4dec.c > > index 457a37e745..d278540ec2 100644 > > --- a/libavcodec/msmpeg4dec.c > > +++ b/libavcodec/msmpeg4dec.c > > @@ -412,6 +412,9 @@ int ff_msmpeg4_decode_picture_header(MpegEncContext * s) > > { > > int code; > > > > + if (get_bits_left(&s->gb) * 8LL < (s->width+15)/16 * ((s->height+15)/16)) > > + return AVERROR_INVALIDDATA; > > + > > Please add a comment so such lines why these magic values where > choosen, and an explanation in the commit message that explains the > proof that these are an absolute limit and no valid frame could ever > be smaller would be appreciated. ill post one with a more verbose description, ill update the 2nd in line with what we agree on for the first thx [...]
diff --git a/libavcodec/msmpeg4dec.c b/libavcodec/msmpeg4dec.c index 457a37e745..d278540ec2 100644 --- a/libavcodec/msmpeg4dec.c +++ b/libavcodec/msmpeg4dec.c @@ -412,6 +412,9 @@ int ff_msmpeg4_decode_picture_header(MpegEncContext * s) { int code; + if (get_bits_left(&s->gb) * 8LL < (s->width+15)/16 * ((s->height+15)/16)) + return AVERROR_INVALIDDATA; + if(s->msmpeg4_version==1){ int start_code = get_bits_long(&s->gb, 32); if(start_code!=0x00000100){
Fixes: Timeout Fixes: 11318/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MSMPEG4V1_fuzzer-5710884555456512 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/msmpeg4dec.c | 3 +++ 1 file changed, 3 insertions(+)