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

[Sean McGovern <gseanmcg@gmail.com>]

On Thu, Nov 2, 2023, 19:50 Michael Niedermayer <michael@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@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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>

On quick inspection this looks OK,  but is s->avctx->width guaranteed to be
non-zero as well?

-- Sean McGovern

>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".