[FFmpeg-devel] avcodec/fic: Check available input space for cursor

Submitted by Michael Niedermayer on May 5, 2018, 8:47 p.m.

Details

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

Commit Message

Michael Niedermayer May 5, 2018, 8:47 p.m.
Fixes: out of array read
Fixes: 6546/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FIC_fuzzer-6317064647081984

Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/fic.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

wm4 May 5, 2018, 9:11 p.m.
On Sat,  5 May 2018 22:47:37 +0200
Michael Niedermayer <michael@niedermayer.cc> wrote:

> Fixes: out of array read
> Fixes: 6546/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FIC_fuzzer-6317064647081984
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/fic.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/libavcodec/fic.c b/libavcodec/fic.c
> index d7ee370423..6824a5683c 100644
> --- a/libavcodec/fic.c
> +++ b/libavcodec/fic.c
> @@ -337,6 +337,11 @@ static int fic_decode_frame(AVCodecContext *avctx, void *data,
>          skip_cursor = 1;
>      }
>  
> +    if (!skip_cursor && avpkt->size < 59 + 32 * 32 * 4) {
> +        av_log(avctx, AV_LOG_WARNING, "Input is cursorless\n");
> +        skip_cursor = 1;
> +    }
> +
>      /* Slice height for all but the last slice. */
>      ctx->slice_h = 16 * (ctx->aligned_height >> 4) / nslices;
>      if (ctx->slice_h % 16)

No warning needed.
Paul B Mahol May 5, 2018, 9:12 p.m.
On 5/5/18, wm4 <nfxjfg@googlemail.com> wrote:
> On Sat,  5 May 2018 22:47:37 +0200
> Michael Niedermayer <michael@niedermayer.cc> wrote:
>
>> Fixes: out of array read
>> Fixes:
>> 6546/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FIC_fuzzer-6317064647081984
>>
>> Found-by: continuous fuzzing process
>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>> ---
>>  libavcodec/fic.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/libavcodec/fic.c b/libavcodec/fic.c
>> index d7ee370423..6824a5683c 100644
>> --- a/libavcodec/fic.c
>> +++ b/libavcodec/fic.c
>> @@ -337,6 +337,11 @@ static int fic_decode_frame(AVCodecContext *avctx,
>> void *data,
>>          skip_cursor = 1;
>>      }
>>
>> +    if (!skip_cursor && avpkt->size < 59 + 32 * 32 * 4) {
>> +        av_log(avctx, AV_LOG_WARNING, "Input is cursorless\n");
>> +        skip_cursor = 1;
>> +    }
>> +
>>      /* Slice height for all but the last slice. */
>>      ctx->slice_h = 16 * (ctx->aligned_height >> 4) / nslices;
>>      if (ctx->slice_h % 16)
>
> No warning needed.

Agreed.
Michael Niedermayer May 5, 2018, 10:47 p.m.
On Sat, May 05, 2018 at 11:12:06PM +0200, Paul B Mahol wrote:
> On 5/5/18, wm4 <nfxjfg@googlemail.com> wrote:
> > On Sat,  5 May 2018 22:47:37 +0200
> > Michael Niedermayer <michael@niedermayer.cc> wrote:
> >
> >> Fixes: out of array read
> >> Fixes:
> >> 6546/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FIC_fuzzer-6317064647081984
> >>
> >> Found-by: continuous fuzzing process
> >> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> >> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> >> ---
> >>  libavcodec/fic.c | 5 +++++
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/libavcodec/fic.c b/libavcodec/fic.c
> >> index d7ee370423..6824a5683c 100644
> >> --- a/libavcodec/fic.c
> >> +++ b/libavcodec/fic.c
> >> @@ -337,6 +337,11 @@ static int fic_decode_frame(AVCodecContext *avctx,
> >> void *data,
> >>          skip_cursor = 1;
> >>      }
> >>
> >> +    if (!skip_cursor && avpkt->size < 59 + 32 * 32 * 4) {
> >> +        av_log(avctx, AV_LOG_WARNING, "Input is cursorless\n");
> >> +        skip_cursor = 1;
> >> +    }
> >> +
> >>      /* Slice height for all but the last slice. */
> >>      ctx->slice_h = 16 * (ctx->aligned_height >> 4) / nslices;
> >>      if (ctx->slice_h % 16)
> >
> > No warning needed.
> 
> Agreed.

Do you prefer i remove the message completely or make it a debug level one ?
Note, it seems every other case that sets skip_cursor in result of a 
unexpected condition prints something

Thanks

[...]
Michael Niedermayer May 16, 2018, 9:11 p.m.
On Sun, May 06, 2018 at 12:47:25AM +0200, Michael Niedermayer wrote:
> On Sat, May 05, 2018 at 11:12:06PM +0200, Paul B Mahol wrote:
> > On 5/5/18, wm4 <nfxjfg@googlemail.com> wrote:
> > > On Sat,  5 May 2018 22:47:37 +0200
> > > Michael Niedermayer <michael@niedermayer.cc> wrote:
> > >
> > >> Fixes: out of array read
> > >> Fixes:
> > >> 6546/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FIC_fuzzer-6317064647081984
> > >>
> > >> Found-by: continuous fuzzing process
> > >> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > >> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > >> ---
> > >>  libavcodec/fic.c | 5 +++++
> > >>  1 file changed, 5 insertions(+)
> > >>
> > >> diff --git a/libavcodec/fic.c b/libavcodec/fic.c
> > >> index d7ee370423..6824a5683c 100644
> > >> --- a/libavcodec/fic.c
> > >> +++ b/libavcodec/fic.c
> > >> @@ -337,6 +337,11 @@ static int fic_decode_frame(AVCodecContext *avctx,
> > >> void *data,
> > >>          skip_cursor = 1;
> > >>      }
> > >>
> > >> +    if (!skip_cursor && avpkt->size < 59 + 32 * 32 * 4) {
> > >> +        av_log(avctx, AV_LOG_WARNING, "Input is cursorless\n");
> > >> +        skip_cursor = 1;
> > >> +    }
> > >> +
> > >>      /* Slice height for all but the last slice. */
> > >>      ctx->slice_h = 16 * (ctx->aligned_height >> 4) / nslices;
> > >>      if (ctx->slice_h % 16)
> > >
> > > No warning needed.
> > 
> > Agreed.
> 
> Do you prefer i remove the message completely or make it a debug level one ?
> Note, it seems every other case that sets skip_cursor in result of a 
> unexpected condition prints something

will apply without the error message

thx

[...]

Patch hide | download patch | download mbox

diff --git a/libavcodec/fic.c b/libavcodec/fic.c
index d7ee370423..6824a5683c 100644
--- a/libavcodec/fic.c
+++ b/libavcodec/fic.c
@@ -337,6 +337,11 @@  static int fic_decode_frame(AVCodecContext *avctx, void *data,
         skip_cursor = 1;
     }
 
+    if (!skip_cursor && avpkt->size < 59 + 32 * 32 * 4) {
+        av_log(avctx, AV_LOG_WARNING, "Input is cursorless\n");
+        skip_cursor = 1;
+    }
+
     /* Slice height for all but the last slice. */
     ctx->slice_h = 16 * (ctx->aligned_height >> 4) / nslices;
     if (ctx->slice_h % 16)