| Message ID | 20231022003520.17154-2-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/6] avformat/mov: Check that is_still_picture_avif has no trak based streams | 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 22/10/2023 01:35, Michael Niedermayer wrote: > Fixes: Assertion length < 256 failed at libavcodec/cbs.c:517 > Fixes: 62673/clusterfuzz-testcase-minimized-ffmpeg_BSF_TRACE_HEADERS_fuzzer-6490971837431808 > > 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.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c > index cdd7adebebd..2f5d0334a2a 100644 > --- a/libavcodec/cbs.c > +++ b/libavcodec/cbs.c > @@ -514,6 +514,11 @@ void ff_cbs_trace_read_log(void *trace_context, > > position = get_bits_count(gbc); > > + if (length >= 256) { > + av_log(ctx->log_ctx, ctx->trace_level, "trace of %d bits truncated at 255\n", length); > + length = 255; > + } > + > av_assert0(length < 256); > for (i = 0; i < length; i++) > bits[i] = get_bits1(gbc) ? '1' : '0'; IMO the assert is sensible (no syntax element is that large) and so this must be catching a bug somewhere else. Please don't nullify the assert to hide the bug. Can you share the input stream which hit this case? Thanks, - Mark
On Sun, Oct 22, 2023 at 03:34:20PM +0100, Mark Thompson wrote: > On 22/10/2023 01:35, Michael Niedermayer wrote: > > Fixes: Assertion length < 256 failed at libavcodec/cbs.c:517 > > Fixes: 62673/clusterfuzz-testcase-minimized-ffmpeg_BSF_TRACE_HEADERS_fuzzer-6490971837431808 > > > > 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.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c > > index cdd7adebebd..2f5d0334a2a 100644 > > --- a/libavcodec/cbs.c > > +++ b/libavcodec/cbs.c > > @@ -514,6 +514,11 @@ void ff_cbs_trace_read_log(void *trace_context, > > position = get_bits_count(gbc); > > + if (length >= 256) { > > + av_log(ctx->log_ctx, ctx->trace_level, "trace of %d bits truncated at 255\n", length); > > + length = 255; > > + } > > + > > av_assert0(length < 256); > > for (i = 0; i < length; i++) > > bits[i] = get_bits1(gbc) ? '1' : '0'; > > IMO the assert is sensible (no syntax element is that large) and so this must be catching a bug somewhere else. Please don't nullify the assert to hide the bug. > > Can you share the input stream which hit this case? will mail it to you the backtrce is this: #7 0x505748 in ff_cbs_trace_read_log ffmpeg/libavcodec/cbs.c:517:5 #8 0x5273f1 in cbs_av1_read_uvlc ffmpeg/libavcodec/cbs_av1.c:67:5 #9 0x5273f1 in cbs_av1_read_timing_info ffmpeg/libavcodec/cbs_av1_syntax_template.c:168 #10 0x5273f1 in cbs_av1_read_sequence_header_obu ffmpeg/libavcodec/cbs_av1_syntax_template.c:214 #11 0x51278a in cbs_av1_read_unit ffmpeg/libavcodec/cbs_av1.c:856:19 #12 0x4ff30a in cbs_read_fragment_content ffmpeg/libavcodec/cbs.c:209:15 #13 0x4ff30a in cbs_read_data ffmpeg/libavcodec/cbs.c:276 #14 0x4edc32 in trace_headers ffmpeg/libavcodec/trace_headers_bsf.c:113:11 #15 0x4c9898 in LLVMFuzzerTestOneInput ffmpeg/tools/target_bsf_fuzzer.c:154:16 #16 0x136900d in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) Fuzzer/build/../FuzzerLoop.cpp:495:13 #17 0x135dbe2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) Fuzzer/build/../FuzzerDriver.cpp:273:6 #18 0x1362de1 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) Fuzzer/build/../FuzzerDriver.cpp:690:9 #19 0x135d8c0 in main Fuzzer/build/../FuzzerMain.cpp:20:10 #20 0x7f456b8b8c86 in __libc_start_main /build/glibc-CVJwZb/glibc-2.27/csu/../csu/libc-start.c:310 #21 0x41f179 in _start (ffmpeg/tools/target_bsf_trace_headers_fuzzer+0x41f179) [...] thx
On 23/10/2023 21:53, Michael Niedermayer wrote: > On Sun, Oct 22, 2023 at 03:34:20PM +0100, Mark Thompson wrote: >> On 22/10/2023 01:35, Michael Niedermayer wrote: >>> Fixes: Assertion length < 256 failed at libavcodec/cbs.c:517 >>> Fixes: 62673/clusterfuzz-testcase-minimized-ffmpeg_BSF_TRACE_HEADERS_fuzzer-6490971837431808 >>> >>> 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.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c >>> index cdd7adebebd..2f5d0334a2a 100644 >>> --- a/libavcodec/cbs.c >>> +++ b/libavcodec/cbs.c >>> @@ -514,6 +514,11 @@ void ff_cbs_trace_read_log(void *trace_context, >>> position = get_bits_count(gbc); >>> + if (length >= 256) { >>> + av_log(ctx->log_ctx, ctx->trace_level, "trace of %d bits truncated at 255\n", length); >>> + length = 255; >>> + } >>> + >>> av_assert0(length < 256); >>> for (i = 0; i < length; i++) >>> bits[i] = get_bits1(gbc) ? '1' : '0'; >> >> IMO the assert is sensible (no syntax element is that large) and so this must be catching a bug somewhere else. Please don't nullify the assert to hide the bug. >> >> Can you share the input stream which hit this case? > > will mail it to you > > the backtrce is this: > > #7 0x505748 in ff_cbs_trace_read_log ffmpeg/libavcodec/cbs.c:517:5 > #8 0x5273f1 in cbs_av1_read_uvlc ffmpeg/libavcodec/cbs_av1.c:67:5 > #9 0x5273f1 in cbs_av1_read_timing_info ffmpeg/libavcodec/cbs_av1_syntax_template.c:168 > #10 0x5273f1 in cbs_av1_read_sequence_header_obu ffmpeg/libavcodec/cbs_av1_syntax_template.c:214 > #11 0x51278a in cbs_av1_read_unit ffmpeg/libavcodec/cbs_av1.c:856:19 > #12 0x4ff30a in cbs_read_fragment_content ffmpeg/libavcodec/cbs.c:209:15 > #13 0x4ff30a in cbs_read_data ffmpeg/libavcodec/cbs.c:276 > #14 0x4edc32 in trace_headers ffmpeg/libavcodec/trace_headers_bsf.c:113:11 > #15 0x4c9898 in LLVMFuzzerTestOneInput ffmpeg/tools/target_bsf_fuzzer.c:154:16 > #16 0x136900d in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) Fuzzer/build/../FuzzerLoop.cpp:495:13 > #17 0x135dbe2 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) Fuzzer/build/../FuzzerDriver.cpp:273:6 > #18 0x1362de1 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) Fuzzer/build/../FuzzerDriver.cpp:690:9 > #19 0x135d8c0 in main Fuzzer/build/../FuzzerMain.cpp:20:10 > #20 0x7f456b8b8c86 in __libc_start_main /build/glibc-CVJwZb/glibc-2.27/csu/../csu/libc-start.c:310 > #21 0x41f179 in _start (ffmpeg/tools/target_bsf_trace_headers_fuzzer+0x41f179) This is the case in <https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2023-October/315983.html>, and would be fixed by that patch. Since the problem is a dubious feature of the standard which other implementations then do not follow I would appreciate thoughts from other people on what to do with it, though. Thanks, - Mark
diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c index cdd7adebebd..2f5d0334a2a 100644 --- a/libavcodec/cbs.c +++ b/libavcodec/cbs.c @@ -514,6 +514,11 @@ void ff_cbs_trace_read_log(void *trace_context, position = get_bits_count(gbc); + if (length >= 256) { + av_log(ctx->log_ctx, ctx->trace_level, "trace of %d bits truncated at 255\n", length); + length = 255; + } + av_assert0(length < 256); for (i = 0; i < length; i++) bits[i] = get_bits1(gbc) ? '1' : '0';
Fixes: Assertion length < 256 failed at libavcodec/cbs.c:517 Fixes: 62673/clusterfuzz-testcase-minimized-ffmpeg_BSF_TRACE_HEADERS_fuzzer-6490971837431808 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.c | 5 +++++ 1 file changed, 5 insertions(+)