| Message ID | 20210130192826.11370-3-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avcodec/dcadsp: Fix integer overflow in dmix_add_c() | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
Quoting Michael Niedermayer (2021-01-30 20:28:26) > Fixes: out of array access > Fixes: 29345/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HAP_fuzzer-5401813482340352 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/hapdec.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/libavcodec/hapdec.c b/libavcodec/hapdec.c > index ab364aa790..260fda2968 100644 > --- a/libavcodec/hapdec.c > +++ b/libavcodec/hapdec.c > @@ -86,6 +86,8 @@ static int hap_parse_decode_instructions(HapContext *ctx, int size) > return ret; > for (i = 0; i < section_size / 4; i++) { > ctx->chunks[i].compressed_offset = bytestream2_get_le32(gbc); > + if (ctx->chunks[i].compressed_offset < 0) Would it not be better to change compressed_offset to uint32 or size_t?
On Thu, Feb 04, 2021 at 12:02:07PM +0100, Anton Khirnov wrote: > Quoting Michael Niedermayer (2021-01-30 20:28:26) > > Fixes: out of array access > > Fixes: 29345/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HAP_fuzzer-5401813482340352 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/hapdec.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/libavcodec/hapdec.c b/libavcodec/hapdec.c > > index ab364aa790..260fda2968 100644 > > --- a/libavcodec/hapdec.c > > +++ b/libavcodec/hapdec.c > > @@ -86,6 +86,8 @@ static int hap_parse_decode_instructions(HapContext *ctx, int size) > > return ret; > > for (i = 0; i < section_size / 4; i++) { > > ctx->chunks[i].compressed_offset = bytestream2_get_le32(gbc); > > + if (ctx->chunks[i].compressed_offset < 0) > > Would it not be better to change compressed_offset to uint32 or size_t? its more work to change the type of a variable as there is more to check. But i agree that a unsigend 32bit should be better (in theory) to hold these 32bit Will post a new patch thx [...]
diff --git a/libavcodec/hapdec.c b/libavcodec/hapdec.c index ab364aa790..260fda2968 100644 --- a/libavcodec/hapdec.c +++ b/libavcodec/hapdec.c @@ -86,6 +86,8 @@ static int hap_parse_decode_instructions(HapContext *ctx, int size) return ret; for (i = 0; i < section_size / 4; i++) { ctx->chunks[i].compressed_offset = bytestream2_get_le32(gbc); + if (ctx->chunks[i].compressed_offset < 0) + return AVERROR_INVALIDDATA; } had_offsets = 1; is_first_table = 0; @@ -106,6 +108,8 @@ static int hap_parse_decode_instructions(HapContext *ctx, int size) for (i = 0; i < ctx->chunk_count; i++) { ctx->chunks[i].compressed_offset = running_size; running_size += ctx->chunks[i].compressed_size; + if (running_size > INT_MAX) + return AVERROR_INVALIDDATA; } }
Fixes: out of array access Fixes: 29345/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HAP_fuzzer-5401813482340352 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/hapdec.c | 4 ++++ 1 file changed, 4 insertions(+)