| Message ID | 20240731195410.274508-4-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/6] avcodec/cbs: sei_3d_reference_displays_info uses length 0 elements | 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 31/07/2024 21:54, Michael Niedermayer wrote: > Fixes: out of array access > Fixes: 70734/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AAC_LATM_fuzzer-4741427068731392 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/aac/aacdec_usac.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/aac/aacdec_usac.c b/libavcodec/aac/aacdec_usac.c > index 82db65eb0d0..2938e693874 100644 > --- a/libavcodec/aac/aacdec_usac.c > +++ b/libavcodec/aac/aacdec_usac.c > @@ -918,8 +918,10 @@ static int decode_usac_stereo_info(AACDecContext *ac, AACUSACConfig *usac, > } > > ret = setup_sce(ac, sce1, usac); > - if (ret < 0) > + if (ret < 0) { > + ics2->max_sfb = 0; > return ret; > + } > > ret = setup_sce(ac, sce2, usac); > if (ret < 0) Err, the one and only place where setup_sce can return an error is also where ics->max_sfb = 0; is cleaned up. It doesn't make sense that this patch would do anything at all.
On Thu, Aug 01, 2024 at 05:11:18PM +0200, Lynne via ffmpeg-devel wrote: > On 31/07/2024 21:54, Michael Niedermayer wrote: > > Fixes: out of array access > > Fixes: 70734/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AAC_LATM_fuzzer-4741427068731392 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/aac/aacdec_usac.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/aac/aacdec_usac.c b/libavcodec/aac/aacdec_usac.c > > index 82db65eb0d0..2938e693874 100644 > > --- a/libavcodec/aac/aacdec_usac.c > > +++ b/libavcodec/aac/aacdec_usac.c > > @@ -918,8 +918,10 @@ static int decode_usac_stereo_info(AACDecContext *ac, AACUSACConfig *usac, > > } > > ret = setup_sce(ac, sce1, usac); > > - if (ret < 0) > > + if (ret < 0) { > > + ics2->max_sfb = 0; > > return ret; > > + } > > ret = setup_sce(ac, sce2, usac); > > if (ret < 0) > > Err, the one and only place where setup_sce can return an error is also > where ics->max_sfb = 0; is cleaned up. It doesn't make sense that this patch > would do anything at all. there are 2 single channel elements when the first fails, it automatically cleans the firsts max_sfb but as is before this patch it leaves the 2nd SCE max_sfb unchanged to whatever unchecked value was put in it. It would get checked and cleared in the next setup_sce() call but that is never called if the first fails thx [...]
On 01/08/2024 19:07, Michael Niedermayer wrote: > On Thu, Aug 01, 2024 at 05:11:18PM +0200, Lynne via ffmpeg-devel wrote: >> On 31/07/2024 21:54, Michael Niedermayer wrote: >>> Fixes: out of array access >>> Fixes: 70734/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AAC_LATM_fuzzer-4741427068731392 >>> >>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>> --- >>> libavcodec/aac/aacdec_usac.c | 4 +++- >>> 1 file changed, 3 insertions(+), 1 deletion(-) >>> >>> diff --git a/libavcodec/aac/aacdec_usac.c b/libavcodec/aac/aacdec_usac.c >>> index 82db65eb0d0..2938e693874 100644 >>> --- a/libavcodec/aac/aacdec_usac.c >>> +++ b/libavcodec/aac/aacdec_usac.c >>> @@ -918,8 +918,10 @@ static int decode_usac_stereo_info(AACDecContext *ac, AACUSACConfig *usac, >>> } >>> ret = setup_sce(ac, sce1, usac); >>> - if (ret < 0) >>> + if (ret < 0) { >>> + ics2->max_sfb = 0; >>> return ret; >>> + } >>> ret = setup_sce(ac, sce2, usac); >>> if (ret < 0) >> >> Err, the one and only place where setup_sce can return an error is also >> where ics->max_sfb = 0; is cleaned up. It doesn't make sense that this patch >> would do anything at all. > > there are 2 single channel elements > when the first fails, it automatically cleans the firsts max_sfb but as is before > this patch it leaves the 2nd SCE max_sfb unchanged to whatever unchecked value > was put in it. > It would get checked and cleared in the next setup_sce() call but that is > never called if the first fails Both patches from the patchset LGTM
diff --git a/libavcodec/aac/aacdec_usac.c b/libavcodec/aac/aacdec_usac.c index 82db65eb0d0..2938e693874 100644 --- a/libavcodec/aac/aacdec_usac.c +++ b/libavcodec/aac/aacdec_usac.c @@ -918,8 +918,10 @@ static int decode_usac_stereo_info(AACDecContext *ac, AACUSACConfig *usac, } ret = setup_sce(ac, sce1, usac); - if (ret < 0) + if (ret < 0) { + ics2->max_sfb = 0; return ret; + } ret = setup_sce(ac, sce2, usac); if (ret < 0)
Fixes: out of array access Fixes: 70734/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AAC_LATM_fuzzer-4741427068731392 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/aac/aacdec_usac.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)