[FFmpeg-devel] [PATCH 3/4] avcodec/flicvideo: consider width in copy loops

[Michael Niedermayer]

On Thu, Nov 02, 2023 at 08:08:41PM -0400, Sean McGovern wrote:
> On Thu, Nov 2, 2023, 19:50 Michael Niedermayer <michael at niedermayer.cc>
> wrote:
> 
> > Fixes: out of array write
> > Fixes:
> > 63520/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FLIC_fuzzer-4876198087622656
> > Regression since: c7f8d42c12582b0626ea38117df6c9aea9fcf5b1 (was not posted
> > to ffmpeg-devel)
> >
> > Found-by: continuous fuzzing process
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by
> > <https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by>:
> > Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavcodec/flicvideo.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavcodec/flicvideo.c b/libavcodec/flicvideo.c
> > index 6ce033ba409..43f3f83bf65 100644
> > --- a/libavcodec/flicvideo.c
> > +++ b/libavcodec/flicvideo.c
> > @@ -642,7 +642,7 @@ static int flic_decode_frame_8BPP(AVCodecContext
> > *avctx,
> >                         "has incorrect size, skipping chunk\n", chunk_size
> > - 6);
> >                  bytestream2_skip(&g2, chunk_size - 6);
> >              } else {
> > -                for (y_ptr = 0; check_pixel_ptr(y_ptr, 0, pixel_limit,
> > direction) == 0;
> > +                for (y_ptr = 0; check_pixel_ptr(y_ptr, s->avctx->width,
> > pixel_limit, direction) == 0;
> >                       y_ptr += s->frame->linesize[0]) {
> >                      bytestream2_get_buffer(&g2, &pixels[y_ptr],
> >                                             s->avctx->width);
> > @@ -949,7 +949,7 @@ static int flic_decode_frame_15_16BPP(AVCodecContext
> > *avctx,
> >
> >                  if (bytestream2_get_bytes_left(&g2) < 2 * s->avctx->width
> > * s->avctx->height )
> >                      return AVERROR_INVALIDDATA;
> > -                for (y_ptr = 0; check_pixel_ptr(y_ptr, 0, pixel_limit,
> > direction) == 0;
> > +                for (y_ptr = 0; check_pixel_ptr(y_ptr, 2*s->avctx->width,
> > pixel_limit, direction) == 0;
> >                       y_ptr += s->frame->linesize[0]) {
> >
> >                      pixel_countdown = s->avctx->width;
> > @@ -1235,7 +1235,7 @@ static int flic_decode_frame_24BPP(AVCodecContext
> > *avctx,
> >                         "bigger than image, skipping chunk\n", chunk_size
> > - 6);
> >                  bytestream2_skip(&g2, chunk_size - 6);
> >              } else {
> > -                for (y_ptr = 0; check_pixel_ptr(y_ptr, 0, pixel_limit,
> > direction) == 0;
> > +                for (y_ptr = 0; check_pixel_ptr(y_ptr, 3*s->avctx->width,
> > pixel_limit, direction) == 0;
> >                       y_ptr += s->frame->linesize[0]) {
> >
> >                      bytestream2_get_buffer(&g2, pixels + y_ptr,
> > 3*s->avctx->width);
> > --
> > 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".
> >
> 
> On quick inspection this looks OK,

will apply patchset (the first 2 with less funny commit messages)


>but is s->avctx->width guaranteed to be
> non-zero as well?

if width somehow manages to be 0 then reget_buffer would fail
so none of this code should be reachable

thx

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

The smallest minority on earth is the individual. Those who deny 
individual rights cannot claim to be defenders of minorities. - Ayn Rand
-------------- 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/20231103/ca7f596e/attachment.sig>