[PATCH 1/3] avcodec/evc_ps: Check chroma_format_idc
Fixes: out of array access Fixes: 62678/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-4858264984354816 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_ps.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libavcodec/evc_ps.c b/libavcodec/evc_ps.c index c11b92d1c5..19895d404d 100644 --- a/libavcodec/evc_ps.c +++ b/libavcodec/evc_ps.c @@ -179,6 +179,10 @@ int ff_evc_parse_sps(GetBitContext *gb, EVCParamSets *ps) // 2 - 4:2:2 // 3 - 4:4:4 sps->chroma_format_idc = get_ue_golomb_31(gb); + if (sps->chroma_format_idc > 3) { + ret = AVERROR_INVALIDDATA; + goto fail; + } sps->pic_width_in_luma_samples = get_ue_golomb_long(gb); sps->pic_height_in_luma_samples = get_ue_golomb_long(gb); -- 2.17.1
The use of float based functions is both unneeded and wrong due to unpredictable rounding Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/evc_parse.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/libavcodec/evc_parse.c b/libavcodec/evc_parse.c index bd3a4416f2..255706ce61 100644 --- a/libavcodec/evc_parse.c +++ b/libavcodec/evc_parse.c @@ -172,7 +172,8 @@ int ff_evc_derive_poc(const EVCParamSets *ps, const EVCParserSliceHeader *sh, poc->PicOrderCntVal = 0; poc->DocOffset = -1; } else { - int SubGopLength = (int)pow(2.0, sps->log2_sub_gop_length); + int SubGopLength = 1 << sps->log2_sub_gop_length; + if (tid == 0) { poc->PicOrderCntVal = poc->prevPicOrderCntVal + SubGopLength; poc->DocOffset = 0; @@ -187,15 +188,16 @@ int ff_evc_derive_poc(const EVCParamSets *ps, const EVCParserSliceHeader *sh, poc->prevPicOrderCntVal += SubGopLength; ExpectedTemporalId = 0; } else - ExpectedTemporalId = 1 + (int)log2(poc->DocOffset); + ExpectedTemporalId = 1 + av_log2(poc->DocOffset); + while (tid != ExpectedTemporalId) { poc->DocOffset = (poc->DocOffset + 1) % SubGopLength; if (poc->DocOffset == 0) ExpectedTemporalId = 0; else - ExpectedTemporalId = 1 + (int)log2(poc->DocOffset); + ExpectedTemporalId = 1 + av_log2(poc->DocOffset); } - PocOffset = (int)(SubGopLength * ((2.0 * poc->DocOffset + 1) / (int)pow(2.0, tid) - 2)); + PocOffset = (int)(SubGopLength * ((2.0 * poc->DocOffset + 1) / (1 << tid) - 2)); poc->PicOrderCntVal = poc->prevPicOrderCntVal + PocOffset; } } -- 2.17.1
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(+) 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
-----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 [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The educated differ from the uneducated as much as the living from the dead. -- Aristotle
On Fri, 13 Oct 2023 at 00:28, Michael Niedermayer <michael@niedermayer.cc> wrote:
Fixes: out of array access Fixes: 62678/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-4858264984354816
Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by <https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by>: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/evc_ps.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/libavcodec/evc_ps.c b/libavcodec/evc_ps.c index c11b92d1c5..19895d404d 100644 --- a/libavcodec/evc_ps.c +++ b/libavcodec/evc_ps.c @@ -179,6 +179,10 @@ int ff_evc_parse_sps(GetBitContext *gb, EVCParamSets *ps) // 2 - 4:2:2 // 3 - 4:4:4 sps->chroma_format_idc = get_ue_golomb_31(gb); + if (sps->chroma_format_idc > 3) { + ret = AVERROR_INVALIDDATA; + goto fail; + }
sps->pic_width_in_luma_samples = get_ue_golomb_long(gb); sps->pic_height_in_luma_samples = get_ue_golomb_long(gb); -- 2.17.1
ok
On Fri, Oct 13, 2023 at 01:18:27AM +0100, Kieran Kunhya wrote:
On Fri, 13 Oct 2023 at 00:28, Michael Niedermayer <michael@niedermayer.cc> wrote:
Fixes: out of array access Fixes: 62678/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-4858264984354816
Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg Signed-off-by <https://github.com/google/oss-fuzz/tree/master/projects/ffmpegSigned-off-by>: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/evc_ps.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/libavcodec/evc_ps.c b/libavcodec/evc_ps.c index c11b92d1c5..19895d404d 100644 --- a/libavcodec/evc_ps.c +++ b/libavcodec/evc_ps.c @@ -179,6 +179,10 @@ int ff_evc_parse_sps(GetBitContext *gb, EVCParamSets *ps) // 2 - 4:2:2 // 3 - 4:4:4 sps->chroma_format_idc = get_ue_golomb_31(gb); + if (sps->chroma_format_idc > 3) { + ret = AVERROR_INVALIDDATA; + goto fail; + }
sps->pic_width_in_luma_samples = get_ue_golomb_long(gb); sps->pic_height_in_luma_samples = get_ue_golomb_long(gb); -- 2.17.1
ok
will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If you fake or manipulate statistics in a paper in physics you will never get a job again. If you fake or manipulate statistics in a paper in medicin you will get a job for life at the pharma industry.
participants (3)
-
Dawid Kozinski/Multimedia (PLT) /SRPOL/Staff Engineer/Samsung Electronics -
Kieran Kunhya -
Michael Niedermayer