| Message ID | 20240401205607.9093-2-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/6] avformat/isom: Uninit layout in ff_mp4_read_dec_config_descr() | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On 4/1/2024 5:56 PM, Michael Niedermayer wrote: > Fixes: null pointer dereference > Fixes: 67494/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6528714521247744 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/mov.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 7bdeeb99f98..fa4c237c0d8 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -9364,6 +9364,10 @@ static int read_image_iovl(AVFormatContext *s, const HEIFGrid *grid, > } > > for (int i = 0; i < tile_grid->nb_tiles; i++) { > + if (!grid->tile_item_list[i]) { > + ret = AVERROR_INVALIDDATA; > + goto fail; > + } This should not happen. We shouldn't get this far if the array was not filled. Can you please test the following? > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 7bdeeb99f9..fb0113b149 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -9397,8 +9397,9 @@ static int mov_parse_tiles(AVFormatContext *s) > > for (int j = 0; j < grid->nb_tiles; j++) { > int tile_id = grid->tile_id_list[j]; > + int k; > > - for (int k = 0; k < mov->nb_heif_item; k++) { > + for (k = 0; k < mov->nb_heif_item; k++) { > HEIFItem *item = &mov->heif_item[k]; > AVStream *st = item->st; > > @@ -9424,6 +9425,13 @@ static int mov_parse_tiles(AVFormatContext *s) > break; > } > > + if (k == grid->nb_tiles) { > + av_log(s, AV_LOG_WARNING, "HEIF item id %d referenced by grid id %d doesn't " > + "exist\n", > + tile_id, grid->item->item_id); > + ff_remove_stream_group(s, stg); > + loop = 0; > + } > if (!loop) > break; > }
On Mon, Apr 01, 2024 at 06:54:35PM -0300, James Almer wrote: > On 4/1/2024 5:56 PM, Michael Niedermayer wrote: > > Fixes: null pointer dereference > > Fixes: 67494/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6528714521247744 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/mov.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index 7bdeeb99f98..fa4c237c0d8 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -9364,6 +9364,10 @@ static int read_image_iovl(AVFormatContext *s, const HEIFGrid *grid, > > } > > for (int i = 0; i < tile_grid->nb_tiles; i++) { > > + if (!grid->tile_item_list[i]) { > > + ret = AVERROR_INVALIDDATA; > > + goto fail; > > + } > > This should not happen. We shouldn't get this far if the array was not > filled. > > Can you please test the following? > > > diff --git a/libavformat/mov.c b/libavformat/mov.c > > index 7bdeeb99f9..fb0113b149 100644 > > --- a/libavformat/mov.c > > +++ b/libavformat/mov.c > > @@ -9397,8 +9397,9 @@ static int mov_parse_tiles(AVFormatContext *s) > > > > for (int j = 0; j < grid->nb_tiles; j++) { > > int tile_id = grid->tile_id_list[j]; > > + int k; > > > > - for (int k = 0; k < mov->nb_heif_item; k++) { > > + for (k = 0; k < mov->nb_heif_item; k++) { > > HEIFItem *item = &mov->heif_item[k]; > > AVStream *st = item->st; > > > > @@ -9424,6 +9425,13 @@ static int mov_parse_tiles(AVFormatContext *s) > > break; > > } > > > > + if (k == grid->nb_tiles) { > > + av_log(s, AV_LOG_WARNING, "HEIF item id %d referenced by grid id %d doesn't " > > + "exist\n", > > + tile_id, grid->item->item_id); > > + ff_remove_stream_group(s, stg); > > + loop = 0; > > + } > > if (!loop) > > break; > > } i confirm the code fixes the issue, please apply (if it passes fate) and backport (if needed) thx [...]
On 4/1/2024 8:49 PM, Michael Niedermayer wrote: > On Mon, Apr 01, 2024 at 06:54:35PM -0300, James Almer wrote: >> On 4/1/2024 5:56 PM, Michael Niedermayer wrote: >>> Fixes: null pointer dereference >>> Fixes: 67494/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6528714521247744 >>> >>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>> --- >>> libavformat/mov.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>> index 7bdeeb99f98..fa4c237c0d8 100644 >>> --- a/libavformat/mov.c >>> +++ b/libavformat/mov.c >>> @@ -9364,6 +9364,10 @@ static int read_image_iovl(AVFormatContext *s, const HEIFGrid *grid, >>> } >>> for (int i = 0; i < tile_grid->nb_tiles; i++) { >>> + if (!grid->tile_item_list[i]) { >>> + ret = AVERROR_INVALIDDATA; >>> + goto fail; >>> + } >> >> This should not happen. We shouldn't get this far if the array was not >> filled. >> >> Can you please test the following? >> >>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>> index 7bdeeb99f9..fb0113b149 100644 >>> --- a/libavformat/mov.c >>> +++ b/libavformat/mov.c >>> @@ -9397,8 +9397,9 @@ static int mov_parse_tiles(AVFormatContext *s) >>> >>> for (int j = 0; j < grid->nb_tiles; j++) { >>> int tile_id = grid->tile_id_list[j]; >>> + int k; >>> >>> - for (int k = 0; k < mov->nb_heif_item; k++) { >>> + for (k = 0; k < mov->nb_heif_item; k++) { >>> HEIFItem *item = &mov->heif_item[k]; >>> AVStream *st = item->st; >>> >>> @@ -9424,6 +9425,13 @@ static int mov_parse_tiles(AVFormatContext *s) >>> break; >>> } >>> >>> + if (k == grid->nb_tiles) { >>> + av_log(s, AV_LOG_WARNING, "HEIF item id %d referenced by grid id %d doesn't " >>> + "exist\n", >>> + tile_id, grid->item->item_id); >>> + ff_remove_stream_group(s, stg); >>> + loop = 0; >>> + } >>> if (!loop) >>> break; >>> } > > i confirm the code fixes the issue, please apply (if it passes fate) and backport (if needed) Applied and backported.
diff --git a/libavformat/mov.c b/libavformat/mov.c index 7bdeeb99f98..fa4c237c0d8 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -9364,6 +9364,10 @@ static int read_image_iovl(AVFormatContext *s, const HEIFGrid *grid, } for (int i = 0; i < tile_grid->nb_tiles; i++) { + if (!grid->tile_item_list[i]) { + ret = AVERROR_INVALIDDATA; + goto fail; + } tile_grid->offsets[i].idx = grid->tile_item_list[i]->st->index; tile_grid->offsets[i].horizontal = (flags & 1) ? avio_rb32(s->pb) : avio_rb16(s->pb); tile_grid->offsets[i].vertical = (flags & 1) ? avio_rb32(s->pb) : avio_rb16(s->pb);
Fixes: null pointer dereference Fixes: 67494/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-6528714521247744 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/mov.c | 4 ++++ 1 file changed, 4 insertions(+)