| Message ID | 20231012232759.5352-3-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | 68cc1744db828e929b74f96478c18f1d226510be |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avcodec/evc_ps: Check chroma_format_idc | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Michael Niedermayer > Sent: piątek, 13 października 2023 01:28 > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Subject: [FFmpeg-devel] [PATCH 3/3] avcodec/evc_parse: Check tid > > The check is based on not infinite looping. It is likely a more strict check can be > done > > Fixes: Infinite loop > Fixes: 62473/clusterfuzz-testcase-minimized- > ffmpeg_BSF_EVC_FRAME_MERGE_fuzzer-5719883750703104 > Fixes: 62765/clusterfuzz-testcase-minimized-ffmpeg_dem_EVC_fuzzer- > 6448531252314112 > > Found-by: continuous fuzzing process > https://protect2.fireeye.com/v1/url?k=06e4faf3-676fefea-06e571bc- > 74fe485cbfec-11816a289a0e9c00&q=1&e=16696cd9-38c1-42d0-9196- > 8ad7c6d1d0d6&u=https%3A%2F%2Fgithub.com%2Fgoogle%2Foss- > fuzz%2Ftree%2Fmaster%2Fprojects%2Fffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/evc_parse.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libavcodec/evc_parse.c b/libavcodec/evc_parse.c index > 255706ce61..43b8dabf8b 100644 > --- a/libavcodec/evc_parse.c > +++ b/libavcodec/evc_parse.c > @@ -174,6 +174,9 @@ int ff_evc_derive_poc(const EVCParamSets *ps, const > EVCParserSliceHeader *sh, > } else { > int SubGopLength = 1 << sps->log2_sub_gop_length; > > + if (tid > (SubGopLength > 1 ? 1 + av_log2(SubGopLength - 1) : 0)) > + return AVERROR_INVALIDDATA; > + > if (tid == 0) { > poc->PicOrderCntVal = poc->prevPicOrderCntVal + SubGopLength; > poc->DocOffset = 0; > -- > 2.17.1 > int SubGopLength = 1 << sps->log2_sub_gop_length; if (tid > 1 + av_log2(SubGopLength - 1)) return AVERROR_INVALIDDATA; For the value of SubGopLength = 1 ( if sps->log2_sub_gop_length = 0; "The value of log2_sub_gop_length shall be in the range of 0 to 5, inclusive" - ISO_IEC_23094-1-2020 7.4.3.1), we have av_log2(0). The value of the logarithm of 0 with any base (in this case, log2) is minus infinity (-inf) Perhaps we should consider changing the condition to: if (tid < 0 || tid > av_log2(SubGopLength)) return AVERROR_INVALIDDATA; > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://protect2.fireeye.com/v1/url?k=63dfcc8a-0254d993-63de47c5- > 74fe485cbfec-e9d44b0bcc16ae00&q=1&e=16696cd9-38c1-42d0-9196- > 8ad7c6d1d0d6&u=https%3A%2F%2Fffmpeg.org%2Fmailman%2Flistinfo%2Fffmp > eg-devel > > To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org > with subject "unsubscribe".
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Michael Niedermayer > Sent: piątek, 13 października 2023 01:28 > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Subject: [FFmpeg-devel] [PATCH 3/3] avcodec/evc_parse: Check tid > > The check is based on not infinite looping. It is likely a more strict check can be > done > > Fixes: Infinite loop > Fixes: 62473/clusterfuzz-testcase-minimized- > ffmpeg_BSF_EVC_FRAME_MERGE_fuzzer-5719883750703104 > Fixes: 62765/clusterfuzz-testcase-minimized-ffmpeg_dem_EVC_fuzzer- > 6448531252314112 > > Found-by: continuous fuzzing process > https://protect2.fireeye.com/v1/url?k=06e4faf3-676fefea-06e571bc- > 74fe485cbfec-11816a289a0e9c00&q=1&e=16696cd9-38c1-42d0-9196- > 8ad7c6d1d0d6&u=https%3A%2F%2Fgithub.com%2Fgoogle%2Foss- > fuzz%2Ftree%2Fmaster%2Fprojects%2Fffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/evc_parse.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libavcodec/evc_parse.c b/libavcodec/evc_parse.c index > 255706ce61..43b8dabf8b 100644 > --- a/libavcodec/evc_parse.c > +++ b/libavcodec/evc_parse.c > @@ -174,6 +174,9 @@ int ff_evc_derive_poc(const EVCParamSets *ps, const > EVCParserSliceHeader *sh, > } else { > int SubGopLength = 1 << sps->log2_sub_gop_length; > > + if (tid > (SubGopLength > 1 ? 1 + av_log2(SubGopLength - 1) : 0)) > + return AVERROR_INVALIDDATA; > + > if (tid == 0) { > poc->PicOrderCntVal = poc->prevPicOrderCntVal + SubGopLength; > poc->DocOffset = 0; > -- > 2.17.1 > My previous comment was related to a different patchset ([FFmpeg-devel,4/4] avcodec/evc_parse: Check tid) I apologize for the mistake > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://protect2.fireeye.com/v1/url?k=63dfcc8a-0254d993-63de47c5- > 74fe485cbfec-e9d44b0bcc16ae00&q=1&e=16696cd9-38c1-42d0-9196- > 8ad7c6d1d0d6&u=https%3A%2F%2Fffmpeg.org%2Fmailman%2Flistinfo%2Fffmp > eg-devel > > To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org > with subject "unsubscribe".
> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > Michael Niedermayer > Sent: piątek, 13 października 2023 01:28 > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Subject: [FFmpeg-devel] [PATCH 3/3] avcodec/evc_parse: Check tid > > The check is based on not infinite looping. It is likely a more strict check can be > done > > Fixes: Infinite loop > Fixes: 62473/clusterfuzz-testcase-minimized- > ffmpeg_BSF_EVC_FRAME_MERGE_fuzzer-5719883750703104 > Fixes: 62765/clusterfuzz-testcase-minimized-ffmpeg_dem_EVC_fuzzer- > 6448531252314112 > > Found-by: continuous fuzzing process > https://protect2.fireeye.com/v1/url?k=06e4faf3-676fefea-06e571bc- > 74fe485cbfec-11816a289a0e9c00&q=1&e=16696cd9-38c1-42d0-9196- > 8ad7c6d1d0d6&u=https%3A%2F%2Fgithub.com%2Fgoogle%2Foss- > fuzz%2Ftree%2Fmaster%2Fprojects%2Fffmpeg > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/evc_parse.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libavcodec/evc_parse.c b/libavcodec/evc_parse.c index > 255706ce61..43b8dabf8b 100644 > --- a/libavcodec/evc_parse.c > +++ b/libavcodec/evc_parse.c > @@ -174,6 +174,9 @@ int ff_evc_derive_poc(const EVCParamSets *ps, const > EVCParserSliceHeader *sh, > } else { > int SubGopLength = 1 << sps->log2_sub_gop_length; > > + if (tid > (SubGopLength > 1 ? 1 + av_log2(SubGopLength - 1) : 0)) > + return AVERROR_INVALIDDATA; > + > if (tid == 0) { > poc->PicOrderCntVal = poc->prevPicOrderCntVal + SubGopLength; > poc->DocOffset = 0; > -- Looks good > 2.17.1 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://protect2.fireeye.com/v1/url?k=63dfcc8a-0254d993-63de47c5- > 74fe485cbfec-e9d44b0bcc16ae00&q=1&e=16696cd9-38c1-42d0-9196- > 8ad7c6d1d0d6&u=https%3A%2F%2Fffmpeg.org%2Fmailman%2Flistinfo%2Fffmp > eg-devel > > To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org > with subject "unsubscribe".
On Fri, Oct 27, 2023 at 03:02:27PM +0200, Dawid Kozinski/Multimedia (PLT) /SRPOL/Staff Engineer/Samsung Electronics wrote: > > > > > > -----Original Message----- > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of > > Michael Niedermayer > > Sent: piątek, 13 października 2023 01:28 > > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > > Subject: [FFmpeg-devel] [PATCH 3/3] avcodec/evc_parse: Check tid > > > > The check is based on not infinite looping. It is likely a more strict > check can be > > done > > > > Fixes: Infinite loop > > Fixes: 62473/clusterfuzz-testcase-minimized- > > ffmpeg_BSF_EVC_FRAME_MERGE_fuzzer-5719883750703104 > > Fixes: 62765/clusterfuzz-testcase-minimized-ffmpeg_dem_EVC_fuzzer- > > 6448531252314112 > > > > Found-by: continuous fuzzing process > > https://protect2.fireeye.com/v1/url?k=06e4faf3-676fefea-06e571bc- > > 74fe485cbfec-11816a289a0e9c00&q=1&e=16696cd9-38c1-42d0-9196- > > 8ad7c6d1d0d6&u=https%3A%2F%2Fgithub.com%2Fgoogle%2Foss- > > fuzz%2Ftree%2Fmaster%2Fprojects%2Fffmpeg > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/evc_parse.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/libavcodec/evc_parse.c b/libavcodec/evc_parse.c index > > 255706ce61..43b8dabf8b 100644 > > --- a/libavcodec/evc_parse.c > > +++ b/libavcodec/evc_parse.c > > @@ -174,6 +174,9 @@ int ff_evc_derive_poc(const EVCParamSets *ps, const > > EVCParserSliceHeader *sh, > > } else { > > int SubGopLength = 1 << sps->log2_sub_gop_length; > > > > + if (tid > (SubGopLength > 1 ? 1 + av_log2(SubGopLength - 1) : > 0)) > > + return AVERROR_INVALIDDATA; > > + > > if (tid == 0) { > > poc->PicOrderCntVal = poc->prevPicOrderCntVal + > SubGopLength; > > poc->DocOffset = 0; > > -- > > > Looks good will apply thx [...]
diff --git a/libavcodec/evc_parse.c b/libavcodec/evc_parse.c index 255706ce61..43b8dabf8b 100644 --- a/libavcodec/evc_parse.c +++ b/libavcodec/evc_parse.c @@ -174,6 +174,9 @@ int ff_evc_derive_poc(const EVCParamSets *ps, const EVCParserSliceHeader *sh, } else { int SubGopLength = 1 << sps->log2_sub_gop_length; + if (tid > (SubGopLength > 1 ? 1 + av_log2(SubGopLength - 1) : 0)) + return AVERROR_INVALIDDATA; + if (tid == 0) { poc->PicOrderCntVal = poc->prevPicOrderCntVal + SubGopLength; poc->DocOffset = 0;
The check is based on not infinite looping. It is likely a more strict check can be done Fixes: Infinite loop Fixes: 62473/clusterfuzz-testcase-minimized-ffmpeg_BSF_EVC_FRAME_MERGE_fuzzer-5719883750703104 Fixes: 62765/clusterfuzz-testcase-minimized-ffmpeg_dem_EVC_fuzzer-6448531252314112 Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/evc_parse.c | 3 +++ 1 file changed, 3 insertions(+)