| Message ID | 20200328211246.30591-1-michael@niedermayer.cc |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec/hapdec: Check tex_size more strictly and before using it | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
On Sat, Mar 28, 2020 at 22:12:45 +0100, Michael Niedermayer wrote: > + if (ctx->tex_size != (avctx->coded_width / TEXTURE_BLOCK_W) > + *(avctx->coded_height / TEXTURE_BLOCK_H) > + *tex_rat[t]) { It would be easier to recognize that this is a multiplication and not a dereference, if the '*' wasn't attached to the right hand term. But I do realize you only moved the block of code. Moritz
On Tue, Mar 31, 2020 at 05:45:39PM +0200, Moritz Barsnick wrote: > On Sat, Mar 28, 2020 at 22:12:45 +0100, Michael Niedermayer wrote: > > + if (ctx->tex_size != (avctx->coded_width / TEXTURE_BLOCK_W) > > + *(avctx->coded_height / TEXTURE_BLOCK_H) > > + *tex_rat[t]) { > > It would be easier to recognize that this is a multiplication and not a > dereference, if the '*' wasn't attached to the right hand term. > > But I do realize you only moved the block of code. ill fix this together with another bug i found in this when i apply or post a new version of this thx [...]
diff --git a/libavcodec/hapdec.c b/libavcodec/hapdec.c index d23ceb5cef..e4abd739c5 100644 --- a/libavcodec/hapdec.c +++ b/libavcodec/hapdec.c @@ -342,6 +342,13 @@ static int hap_decode(AVCodecContext *avctx, void *data, if (ret < 0) return ret; + if (ctx->tex_size != (avctx->coded_width / TEXTURE_BLOCK_W) + *(avctx->coded_height / TEXTURE_BLOCK_H) + *tex_rat[t]) { + av_log(avctx, AV_LOG_ERROR, "uncompressed size mismatches\n"); + return AVERROR_INVALIDDATA; + } + start_texture_section += ctx->texture_section_size + 4; if (avctx->codec->update_thread_context) @@ -370,13 +377,6 @@ static int hap_decode(AVCodecContext *avctx, void *data, tex_size = ctx->tex_size; } - if (tex_size < (avctx->coded_width / TEXTURE_BLOCK_W) - *(avctx->coded_height / TEXTURE_BLOCK_H) - *tex_rat[t]) { - av_log(avctx, AV_LOG_ERROR, "Insufficient data\n"); - return AVERROR_INVALIDDATA; - } - /* Use the decompress function on the texture, one block per thread */ if (t == 0){ avctx->execute2(avctx, decompress_texture_thread, tframe.f, NULL, ctx->slice_count);
Fixes: OOM Fixes: 20774/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HAP_fuzzer-5678608951803904 Fixes: 20956/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_HAP_fuzzer-5713643025203200 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 | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)