| Message ID | 20211113210447.28681-1-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/vqavideo: Use GetByteContext and check for end | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
On Sat, Nov 13, 2021 at 10:04:47PM +0100, Michael Niedermayer wrote: > Fixes: out of array access > Fixes: Timeout > Fixes: 40481/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VQA_fuzzer-6502647583080448 > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/vqavideo.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) will apply [...]
Michael Niedermayer: > Fixes: out of array access > Fixes: Timeout > Fixes: 40481/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VQA_fuzzer-6502647583080448 > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/vqavideo.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/libavcodec/vqavideo.c b/libavcodec/vqavideo.c > index 5466e25cdf1..755abf6bafa 100644 > --- a/libavcodec/vqavideo.c > +++ b/libavcodec/vqavideo.c > @@ -633,7 +633,7 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame) > int vptr_chunk = -1; > int vprz_chunk = -1; > > - const unsigned char *stream; > + GetByteContext gb_stream; > > while (bytestream2_get_bytes_left(&s->gb) >= 8) { > chunk_type = bytestream2_get_be32u(&s->gb); > @@ -722,7 +722,7 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame) > > /* now uncompress the per-row RLE of the decode buffer and draw the blocks in framebuffer */ > > - stream = (unsigned char*)s->decode_buffer; > + bytestream2_init(&gb_stream, s->decode_buffer, s->decode_buffer_size); > > for (int y_pos = 0; y_pos < s->height; y_pos += s->vector_height) { > int x_pos = 0; > @@ -730,9 +730,14 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame) > while (x_pos < s->width) { > int vector_index = 0; > int count = 0; > - uint16_t code = bytestream_get_le16(&stream); > + uint16_t code; > int type; > > + if (bytestream2_get_bytes_left(&gb_stream) < 1) Why are you only checking for one byte to be present although you read two bytes immediately afterwards? > + return AVERROR_INVALIDDATA; > + > + code = bytestream2_get_le16(&gb_stream); > + > type = code >> 13; > code &= 0x1fff; > > @@ -747,7 +752,7 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame) > count = 1; > } else if (type < 7) { > vector_index = code; > - count = *stream++; > + count = bytestream2_get_byte(&gb_stream); > } else { > av_log(s->avctx, AV_LOG_ERROR, " unknown type in VPTR chunk (%d)\n",type); > return AVERROR_INVALIDDATA; > @@ -776,7 +781,7 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame) > > /* we might want to read the next block index from stream */ > if ((type == 2) && count > 0) { > - vector_index = bytestream_get_byte(&stream); > + vector_index = bytestream2_get_byte(&gb_stream); > } > > x_pos += 4; >
On Mon, Nov 29, 2021 at 04:00:27PM +0100, Andreas Rheinhardt wrote: > Michael Niedermayer: > > Fixes: out of array access > > Fixes: Timeout > > Fixes: 40481/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VQA_fuzzer-6502647583080448 > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/vqavideo.c | 15 ++++++++++----- > > 1 file changed, 10 insertions(+), 5 deletions(-) > > > > diff --git a/libavcodec/vqavideo.c b/libavcodec/vqavideo.c > > index 5466e25cdf1..755abf6bafa 100644 > > --- a/libavcodec/vqavideo.c > > +++ b/libavcodec/vqavideo.c > > @@ -633,7 +633,7 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame) > > int vptr_chunk = -1; > > int vprz_chunk = -1; > > > > - const unsigned char *stream; > > + GetByteContext gb_stream; > > > > while (bytestream2_get_bytes_left(&s->gb) >= 8) { > > chunk_type = bytestream2_get_be32u(&s->gb); > > @@ -722,7 +722,7 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame) > > > > /* now uncompress the per-row RLE of the decode buffer and draw the blocks in framebuffer */ > > > > - stream = (unsigned char*)s->decode_buffer; > > + bytestream2_init(&gb_stream, s->decode_buffer, s->decode_buffer_size); > > > > for (int y_pos = 0; y_pos < s->height; y_pos += s->vector_height) { > > int x_pos = 0; > > @@ -730,9 +730,14 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame) > > while (x_pos < s->width) { > > int vector_index = 0; > > int count = 0; > > - uint16_t code = bytestream_get_le16(&stream); > > + uint16_t code; > > int type; > > > > + if (bytestream2_get_bytes_left(&gb_stream) < 1) > > Why are you only checking for one byte to be present although you read > two bytes immediately afterwards? because i apparently cannot count to 2 i will fix that before applying thx [...]
diff --git a/libavcodec/vqavideo.c b/libavcodec/vqavideo.c index 5466e25cdf1..755abf6bafa 100644 --- a/libavcodec/vqavideo.c +++ b/libavcodec/vqavideo.c @@ -633,7 +633,7 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame) int vptr_chunk = -1; int vprz_chunk = -1; - const unsigned char *stream; + GetByteContext gb_stream; while (bytestream2_get_bytes_left(&s->gb) >= 8) { chunk_type = bytestream2_get_be32u(&s->gb); @@ -722,7 +722,7 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame) /* now uncompress the per-row RLE of the decode buffer and draw the blocks in framebuffer */ - stream = (unsigned char*)s->decode_buffer; + bytestream2_init(&gb_stream, s->decode_buffer, s->decode_buffer_size); for (int y_pos = 0; y_pos < s->height; y_pos += s->vector_height) { int x_pos = 0; @@ -730,9 +730,14 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame) while (x_pos < s->width) { int vector_index = 0; int count = 0; - uint16_t code = bytestream_get_le16(&stream); + uint16_t code; int type; + if (bytestream2_get_bytes_left(&gb_stream) < 1) + return AVERROR_INVALIDDATA; + + code = bytestream2_get_le16(&gb_stream); + type = code >> 13; code &= 0x1fff; @@ -747,7 +752,7 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame) count = 1; } else if (type < 7) { vector_index = code; - count = *stream++; + count = bytestream2_get_byte(&gb_stream); } else { av_log(s->avctx, AV_LOG_ERROR, " unknown type in VPTR chunk (%d)\n",type); return AVERROR_INVALIDDATA; @@ -776,7 +781,7 @@ static int vqa_decode_frame_hicolor(VqaContext *s, AVFrame *frame) /* we might want to read the next block index from stream */ if ((type == 2) && count > 0) { - vector_index = bytestream_get_byte(&stream); + vector_index = bytestream2_get_byte(&gb_stream); } x_pos += 4;
Fixes: out of array access Fixes: Timeout Fixes: 40481/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VQA_fuzzer-6502647583080448 Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/vqavideo.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)