| Message ID | 20240426235211.3718252-3-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avcodec/pngdec: Check last AVFrame before deref | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
Michael Niedermayer: > Fixes: NULL pointer dereference > Fixes: 68192/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP8_fuzzer-6180311026171904 > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/decode.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavcodec/decode.c b/libavcodec/decode.c > index d031b1ca176..a6131941f43 100644 > --- a/libavcodec/decode.c > +++ b/libavcodec/decode.c > @@ -1744,6 +1744,8 @@ void ff_progress_frame_report(ProgressFrame *f, int n) > > void ff_progress_frame_await(const ProgressFrame *f, int n) > { > + if (!f->progress) > + return; > ff_thread_progress_await(&f->progress->progress, n); > } > Can I get the sample? I see two places in VP8 where the VP8Frame pointers are set before the actual frame inside it is properly allocated. (Actually, it was intended for this API to not support waiting on non-existent frames (i.e. let the caller check for this; in most instances, it is already guaranteed that the frame one waits one exists, so this is unnecessary for them).) - Andreas
On Sat, Apr 27, 2024 at 01:13:54PM +0200, Andreas Rheinhardt wrote: > Michael Niedermayer: > > Fixes: NULL pointer dereference > > Fixes: 68192/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP8_fuzzer-6180311026171904 > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/decode.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/libavcodec/decode.c b/libavcodec/decode.c > > index d031b1ca176..a6131941f43 100644 > > --- a/libavcodec/decode.c > > +++ b/libavcodec/decode.c > > @@ -1744,6 +1744,8 @@ void ff_progress_frame_report(ProgressFrame *f, int n) > > > > void ff_progress_frame_await(const ProgressFrame *f, int n) > > { > > + if (!f->progress) > > + return; > > ff_thread_progress_await(&f->progress->progress, n); > > } > > > > Can I get the sample? I see two places in VP8 where the VP8Frame > pointers are set before the actual frame inside it is properly allocated. > > (Actually, it was intended for this API to not support waiting on > non-existent frames (i.e. let the caller check for this; in most > instances, it is already guaranteed that the frame one waits one exists, > so this is unnecessary for them).) did you receive the sample i sent you in april ? any update on this ? its still crashing without this patch Running: 68192/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP8_fuzzer-6180311026171904 libavcodec/decode.c:1766:44: runtime error: member access within null pointer of type 'struct ProgressInternal' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior libavcodec/decode.c:1766:44 in libavcodec/threadprogress.c:72:36: runtime error: member access within null pointer of type 'ThreadProgress' (aka 'struct ThreadProgress') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior libavcodec/threadprogress.c:72:36 in thx [...]
Michael Niedermayer: > On Sat, Apr 27, 2024 at 01:13:54PM +0200, Andreas Rheinhardt wrote: >> Michael Niedermayer: >>> Fixes: NULL pointer dereference >>> Fixes: 68192/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP8_fuzzer-6180311026171904 >>> >>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>> --- >>> libavcodec/decode.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c >>> index d031b1ca176..a6131941f43 100644 >>> --- a/libavcodec/decode.c >>> +++ b/libavcodec/decode.c >>> @@ -1744,6 +1744,8 @@ void ff_progress_frame_report(ProgressFrame *f, int n) >>> >>> void ff_progress_frame_await(const ProgressFrame *f, int n) >>> { >>> + if (!f->progress) >>> + return; >>> ff_thread_progress_await(&f->progress->progress, n); >>> } >>> >> >> Can I get the sample? I see two places in VP8 where the VP8Frame >> pointers are set before the actual frame inside it is properly allocated. >> >> (Actually, it was intended for this API to not support waiting on >> non-existent frames (i.e. let the caller check for this; in most >> instances, it is already guaranteed that the frame one waits one exists, >> so this is unnecessary for them).) > > did you receive the sample i sent you in april ? > > any update on this ? > its still crashing without this patch > > Running: 68192/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP8_fuzzer-6180311026171904 > libavcodec/decode.c:1766:44: runtime error: member access within null pointer of type 'struct ProgressInternal' > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior libavcodec/decode.c:1766:44 in > libavcodec/threadprogress.c:72:36: runtime error: member access within null pointer of type 'ThreadProgress' (aka 'struct ThreadProgress') > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior libavcodec/threadprogress.c:72:36 in > Totally forgot about this. Will look into it. Thanks for the reminder. - Andreas
On Tue, Jun 25, 2024 at 09:51:32PM +0200, Andreas Rheinhardt wrote: > Michael Niedermayer: > > On Sat, Apr 27, 2024 at 01:13:54PM +0200, Andreas Rheinhardt wrote: > >> Michael Niedermayer: > >>> Fixes: NULL pointer dereference > >>> Fixes: 68192/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP8_fuzzer-6180311026171904 > >>> > >>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > >>> --- > >>> libavcodec/decode.c | 2 ++ > >>> 1 file changed, 2 insertions(+) > >>> > >>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c > >>> index d031b1ca176..a6131941f43 100644 > >>> --- a/libavcodec/decode.c > >>> +++ b/libavcodec/decode.c > >>> @@ -1744,6 +1744,8 @@ void ff_progress_frame_report(ProgressFrame *f, int n) > >>> > >>> void ff_progress_frame_await(const ProgressFrame *f, int n) > >>> { > >>> + if (!f->progress) > >>> + return; > >>> ff_thread_progress_await(&f->progress->progress, n); > >>> } > >>> > >> > >> Can I get the sample? I see two places in VP8 where the VP8Frame > >> pointers are set before the actual frame inside it is properly allocated. > >> > >> (Actually, it was intended for this API to not support waiting on > >> non-existent frames (i.e. let the caller check for this; in most > >> instances, it is already guaranteed that the frame one waits one exists, > >> so this is unnecessary for them).) > > > > did you receive the sample i sent you in april ? > > > > any update on this ? > > its still crashing without this patch > > > > Running: 68192/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP8_fuzzer-6180311026171904 > > libavcodec/decode.c:1766:44: runtime error: member access within null pointer of type 'struct ProgressInternal' > > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior libavcodec/decode.c:1766:44 in > > libavcodec/threadprogress.c:72:36: runtime error: member access within null pointer of type 'ThreadProgress' (aka 'struct ThreadProgress') > > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior libavcodec/threadprogress.c:72:36 in > > > > Totally forgot about this. Will look into it. Thanks for the reminder. Hello ? this issue is still open thx [...]
diff --git a/libavcodec/decode.c b/libavcodec/decode.c index d031b1ca176..a6131941f43 100644 --- a/libavcodec/decode.c +++ b/libavcodec/decode.c @@ -1744,6 +1744,8 @@ void ff_progress_frame_report(ProgressFrame *f, int n) void ff_progress_frame_await(const ProgressFrame *f, int n) { + if (!f->progress) + return; ff_thread_progress_await(&f->progress->progress, n); }
Fixes: NULL pointer dereference Fixes: 68192/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VP8_fuzzer-6180311026171904 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/decode.c | 2 ++ 1 file changed, 2 insertions(+)