| Message ID | 20190804164416.23266-1-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | 8e4b522c9146b9c14579ae7381fb1043b7423578 |
| Headers | show |
On Sun, Aug 04, 2019 at 06:44:04PM +0200, Michael Niedermayer wrote: > Fixes: Assertion failure and memleak > Fixes: 15709/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APE_fuzzer-5182435093905408 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/apedec.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) will apply patchset and backport to release/4.2 soon [...]
On 8/4/2019 1:44 PM, Michael Niedermayer wrote: > Fixes: index -1 out of bounds for type 'AV1ReferenceFrameState [8]' > Fixes: 16079/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5758807440883712 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/cbs_av1_syntax_template.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c > index b04cd51d55..806b302de6 100644 > --- a/libavcodec/cbs_av1_syntax_template.c > +++ b/libavcodec/cbs_av1_syntax_template.c > @@ -419,16 +419,17 @@ static int FUNC(frame_size_with_refs)(CodedBitstreamContext *ctx, RWContext *rw, > for (i = 0; i < AV1_REFS_PER_FRAME; i++) { > flags(found_ref[i], 1, i); > if (current->found_ref[i]) { > - AV1ReferenceFrameState *ref = > - &priv->ref[current->ref_frame_idx[i]]; > + AV1ReferenceFrameState *ref; > > - if (!ref->valid) { > + if (current->ref_frame_idx[i] < 0 || > + !priv->ref[current->ref_frame_idx[i]].valid) { > av_log(ctx->log_ctx, AV_LOG_ERROR, > "Missing reference frame needed for frame size " > "(ref = %d, ref_frame_idx = %d).\n", > i, current->ref_frame_idx[i]); > return AVERROR_INVALIDDATA; > } > + ref = &priv->ref[current->ref_frame_idx[i]]; > > priv->upscaled_width = ref->upscaled_width; > priv->frame_width = ref->frame_width; This actually revealed a bug when setting ref_frame_idx[i] in the frame_refs_short_signaling == true code path. It's incomplete given that the -1 is a placeholder meant to be replaced further into the process. This change is ok to prevent the out of bounds issue for now, but valid files are in theory being rejected, and that should be fixed.
On Mon, Aug 05, 2019 at 12:19:24PM -0300, James Almer wrote: > On 8/4/2019 1:44 PM, Michael Niedermayer wrote: > > Fixes: index -1 out of bounds for type 'AV1ReferenceFrameState [8]' > > Fixes: 16079/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5758807440883712 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/cbs_av1_syntax_template.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c > > index b04cd51d55..806b302de6 100644 > > --- a/libavcodec/cbs_av1_syntax_template.c > > +++ b/libavcodec/cbs_av1_syntax_template.c > > @@ -419,16 +419,17 @@ static int FUNC(frame_size_with_refs)(CodedBitstreamContext *ctx, RWContext *rw, > > for (i = 0; i < AV1_REFS_PER_FRAME; i++) { > > flags(found_ref[i], 1, i); > > if (current->found_ref[i]) { > > - AV1ReferenceFrameState *ref = > > - &priv->ref[current->ref_frame_idx[i]]; > > + AV1ReferenceFrameState *ref; > > > > - if (!ref->valid) { > > + if (current->ref_frame_idx[i] < 0 || > > + !priv->ref[current->ref_frame_idx[i]].valid) { > > av_log(ctx->log_ctx, AV_LOG_ERROR, > > "Missing reference frame needed for frame size " > > "(ref = %d, ref_frame_idx = %d).\n", > > i, current->ref_frame_idx[i]); > > return AVERROR_INVALIDDATA; > > } > > + ref = &priv->ref[current->ref_frame_idx[i]]; > > > > priv->upscaled_width = ref->upscaled_width; > > priv->frame_width = ref->frame_width; > > This actually revealed a bug when setting ref_frame_idx[i] in the > frame_refs_short_signaling == true code path. It's incomplete given that > the -1 is a placeholder meant to be replaced further into the process. > > This change is ok to prevent the out of bounds issue for now, but valid > files are in theory being rejected, and that should be fixed. ok, will apply then. Thanks [...]
diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c index e9ffdfdcdf..8872185b84 100644 --- a/libavcodec/apedec.c +++ b/libavcodec/apedec.c @@ -1451,7 +1451,8 @@ static int ape_decode_frame(AVCodecContext *avctx, void *data, if (s->fileversion >= 3900) { if (offset > 3) { av_log(avctx, AV_LOG_ERROR, "Incorrect offset passed\n"); - s->data = NULL; + av_freep(&s->data); + s->data_size = 0; return AVERROR_INVALIDDATA; } if (s->data_end - s->ptr < offset) {
Fixes: Assertion failure and memleak Fixes: 15709/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_APE_fuzzer-5182435093905408 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/apedec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)