| Message ID | 20231102235016.3935-3-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | 03a4aa9699c397f157394af3394fb065bd0a8166 |
| Headers | show |
| Series | [FFmpeg-devel,1/4] avfilter/framesync: cuddle () closer around = | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
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 >
On Thu, Nov 02, 2023 at 08:08:41PM -0400, Sean McGovern wrote: > 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, 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 [...]
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);
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: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/flicvideo.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)