[FFmpeg-devel,1/2] avcodec/msmpeg4dec: Skip frame if its smaller than 1/8 of the minimal size

Submitted by Michael Niedermayer on Nov. 28, 2018, 12:46 a.m.

Details

Message ID 20181128004650.30626-1-michael@niedermayer.cc
State New
Headers show

Commit Message

Michael Niedermayer Nov. 28, 2018, 12:46 a.m.
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(+)

Comments

Hendrik Leppkes Nov. 28, 2018, 9:06 a.m.
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
Michael Niedermayer Nov. 29, 2018, 1:32 a.m.
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

[...]

Patch hide | download patch | download mbox

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){