| Message ID | 20200924202039.30285-2-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/4] avcodec/svq3: dont crash on free_picture(NULL) | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
On 9/24/2020 5:20 PM, Michael Niedermayer wrote: > Fixes: member access within null pointer of type 'TileGroupInfo' (aka 'struct TileGroupInfo') > Fixes: 25725/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AV1_fuzzer-5166692706287616 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/av1dec.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c > index 0bb04a3e44..cf3a78aad8 100644 > --- a/libavcodec/av1dec.c > +++ b/libavcodec/av1dec.c > @@ -209,6 +209,9 @@ static int get_tiles_info(AVCodecContext *avctx, const AV1RawTileGroup *tile_gro > uint16_t tile_num, tile_row, tile_col; > uint32_t size = 0, size_bytes = 0; > > + if (s->tile_num != s->raw_frame_header->tile_cols * s->raw_frame_header->tile_rows) > + return AVERROR_INVALIDDATA; This shouldn't happen if a frame header was properly parsed. It sounds like one wasn't yet s->raw_frame_header was left pointing to it. Does the following also fix this crash? > diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c > index 0bb04a3e44..e650d43d9e 100644 > --- a/libavcodec/av1dec.c > +++ b/libavcodec/av1dec.c > @@ -831,6 +831,9 @@ static int av1_decode_frame(AVCodecContext *avctx, void *frame, > > end: > ff_cbs_fragment_reset(&s->current_obu); > + if (ret < 0) > + s->raw_frame_header = NULL; > + > return ret; > } >
On Thu, Sep 24, 2020 at 05:51:43PM -0300, James Almer wrote: > On 9/24/2020 5:20 PM, Michael Niedermayer wrote: > > Fixes: member access within null pointer of type 'TileGroupInfo' (aka 'struct TileGroupInfo') > > Fixes: 25725/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AV1_fuzzer-5166692706287616 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/av1dec.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c > > index 0bb04a3e44..cf3a78aad8 100644 > > --- a/libavcodec/av1dec.c > > +++ b/libavcodec/av1dec.c > > @@ -209,6 +209,9 @@ static int get_tiles_info(AVCodecContext *avctx, const AV1RawTileGroup *tile_gro > > uint16_t tile_num, tile_row, tile_col; > > uint32_t size = 0, size_bytes = 0; > > > > + if (s->tile_num != s->raw_frame_header->tile_cols * s->raw_frame_header->tile_rows) > > + return AVERROR_INVALIDDATA; > > This shouldn't happen if a frame header was properly parsed. It sounds > like one wasn't yet s->raw_frame_header was left pointing to it. > > Does the following also fix this crash? I was considering to clear raw_frame_header but went for the smaller change as it wasnt clear to me if that would have unintended side effects / and wondered why it was not cleared on any error path ... But yes if clearing it is ok that is better and i confirm the suggested change works thx [...]
On 9/25/2020 5:05 AM, Michael Niedermayer wrote: > On Thu, Sep 24, 2020 at 05:51:43PM -0300, James Almer wrote: >> On 9/24/2020 5:20 PM, Michael Niedermayer wrote: >>> Fixes: member access within null pointer of type 'TileGroupInfo' (aka 'struct TileGroupInfo') >>> Fixes: 25725/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AV1_fuzzer-5166692706287616 >>> >>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>> --- >>> libavcodec/av1dec.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c >>> index 0bb04a3e44..cf3a78aad8 100644 >>> --- a/libavcodec/av1dec.c >>> +++ b/libavcodec/av1dec.c >>> @@ -209,6 +209,9 @@ static int get_tiles_info(AVCodecContext *avctx, const AV1RawTileGroup *tile_gro >>> uint16_t tile_num, tile_row, tile_col; >>> uint32_t size = 0, size_bytes = 0; >>> >>> + if (s->tile_num != s->raw_frame_header->tile_cols * s->raw_frame_header->tile_rows) >>> + return AVERROR_INVALIDDATA; >> >> This shouldn't happen if a frame header was properly parsed. It sounds >> like one wasn't yet s->raw_frame_header was left pointing to it. >> > >> Does the following also fix this crash? > > I was considering to clear raw_frame_header but went for the smaller change > as it wasnt clear to me if that would have unintended side effects / and > wondered why it was not cleared on any error path ... > But yes if clearing it is ok that is better > and i confirm the suggested change works Yes, it's an oversight. Will send a patchset to make some more thorough cleaning on error, including this fix.
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c index 0bb04a3e44..cf3a78aad8 100644 --- a/libavcodec/av1dec.c +++ b/libavcodec/av1dec.c @@ -209,6 +209,9 @@ static int get_tiles_info(AVCodecContext *avctx, const AV1RawTileGroup *tile_gro uint16_t tile_num, tile_row, tile_col; uint32_t size = 0, size_bytes = 0; + if (s->tile_num != s->raw_frame_header->tile_cols * s->raw_frame_header->tile_rows) + return AVERROR_INVALIDDATA; + bytestream2_init(&gb, tile_group->tile_data.data, tile_group->tile_data.data_size); s->tg_start = tile_group->tg_start;
Fixes: member access within null pointer of type 'TileGroupInfo' (aka 'struct TileGroupInfo') Fixes: 25725/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AV1_fuzzer-5166692706287616 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/av1dec.c | 3 +++ 1 file changed, 3 insertions(+)