| Message ID | 20240919225639.2376418-5-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On 9/19/2024 7:56 PM, Michael Niedermayer wrote: > Fixes: shift exponent 62 is too large for 32-bit type 'int' > Fixes: 71020/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VVC_fuzzer-6444916325023744 > > 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_h266_syntax_template.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/libavcodec/cbs_h266_syntax_template.c b/libavcodec/cbs_h266_syntax_template.c > index a8f5af04d02..1c111126563 100644 > --- a/libavcodec/cbs_h266_syntax_template.c > +++ b/libavcodec/cbs_h266_syntax_template.c > @@ -1041,6 +1041,9 @@ static int FUNC(sps_range_extension)(CodedBitstreamContext *ctx, RWContext *rw, > { > int err; > > + if (current->sps_bitdepth_minus8 < 10) sps_bitdepth_minus8 can only be between 0 and 8, so this is basically making it abort on any and every sample with SPS range extension. Also, it doesn't seem to be used here at all, so i don't see how this is fixing anything. Can you share the sample? > + return AVERROR_INVALIDDATA; > + > flag(sps_extended_precision_flag); > if (current->sps_transform_skip_enabled_flag) > flag(sps_ts_residual_coding_rice_present_in_sh_flag);
On Thu, Sep 19, 2024 at 08:53:07PM -0300, James Almer wrote: > On 9/19/2024 7:56 PM, Michael Niedermayer wrote: > > Fixes: shift exponent 62 is too large for 32-bit type 'int' > > Fixes: 71020/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VVC_fuzzer-6444916325023744 > > > > 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_h266_syntax_template.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/libavcodec/cbs_h266_syntax_template.c b/libavcodec/cbs_h266_syntax_template.c > > index a8f5af04d02..1c111126563 100644 > > --- a/libavcodec/cbs_h266_syntax_template.c > > +++ b/libavcodec/cbs_h266_syntax_template.c > > @@ -1041,6 +1041,9 @@ static int FUNC(sps_range_extension)(CodedBitstreamContext *ctx, RWContext *rw, > > { > > int err; > > + if (current->sps_bitdepth_minus8 < 10) > > sps_bitdepth_minus8 can only be between 0 and 8, so this is basically making > it abort on any and every sample with SPS range extension. + if (current->sps_bitdepth_minus8 < 10 - 8) Its supposed to check this: "When BitDepth is less than or equal to 10, the value of sps_range_extension_flag shall be equal to 0." > > Also, it doesn't seem to be used here at all, so i don't see how this is > fixing anything. Can you share the sample? its used here: libavcodec/vvc/ctu.c: persistent_rice_adaptation_enabled_flag ? 2 * (av_log2(bit_depth - 10)) : 0; I can send you the file but i think this case is quite clear thx [...]
On Fri, Sep 20, 2024 at 02:34:25AM +0200, Michael Niedermayer wrote: > On Thu, Sep 19, 2024 at 08:53:07PM -0300, James Almer wrote: > > On 9/19/2024 7:56 PM, Michael Niedermayer wrote: > > > Fixes: shift exponent 62 is too large for 32-bit type 'int' > > > Fixes: 71020/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VVC_fuzzer-6444916325023744 > > > > > > 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_h266_syntax_template.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/libavcodec/cbs_h266_syntax_template.c b/libavcodec/cbs_h266_syntax_template.c > > > index a8f5af04d02..1c111126563 100644 > > > --- a/libavcodec/cbs_h266_syntax_template.c > > > +++ b/libavcodec/cbs_h266_syntax_template.c > > > @@ -1041,6 +1041,9 @@ static int FUNC(sps_range_extension)(CodedBitstreamContext *ctx, RWContext *rw, > > > { > > > int err; > > > + if (current->sps_bitdepth_minus8 < 10) > > > > sps_bitdepth_minus8 can only be between 0 and 8, so this is basically making > > it abort on any and every sample with SPS range extension. > > + if (current->sps_bitdepth_minus8 < 10 - 8) > > Its supposed to check this: > "When BitDepth is less > than or equal to 10, the value of sps_range_extension_flag shall be equal to 0." also on this subject, using variables like var_minusconstant leads to mistakes it would be better to do var = read() + constant write(var - constant) it results in cleaner code IMHO thx [...]
On 9/19/2024 9:34 PM, Michael Niedermayer wrote: > On Thu, Sep 19, 2024 at 08:53:07PM -0300, James Almer wrote: >> On 9/19/2024 7:56 PM, Michael Niedermayer wrote: >>> Fixes: shift exponent 62 is too large for 32-bit type 'int' >>> Fixes: 71020/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VVC_fuzzer-6444916325023744 >>> >>> 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_h266_syntax_template.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/libavcodec/cbs_h266_syntax_template.c b/libavcodec/cbs_h266_syntax_template.c >>> index a8f5af04d02..1c111126563 100644 >>> --- a/libavcodec/cbs_h266_syntax_template.c >>> +++ b/libavcodec/cbs_h266_syntax_template.c >>> @@ -1041,6 +1041,9 @@ static int FUNC(sps_range_extension)(CodedBitstreamContext *ctx, RWContext *rw, >>> { >>> int err; >>> + if (current->sps_bitdepth_minus8 < 10) >> >> sps_bitdepth_minus8 can only be between 0 and 8, so this is basically making >> it abort on any and every sample with SPS range extension. > > + if (current->sps_bitdepth_minus8 < 10 - 8) Ok, this is different. > > Its supposed to check this: > "When BitDepth is less > than or equal to 10, the value of sps_range_extension_flag shall be equal to 0." Should be "<= (10 - 8)" then, and LGTM. > > >> >> Also, it doesn't seem to be used here at all, so i don't see how this is >> fixing anything. Can you share the sample? > > its used here: > libavcodec/vvc/ctu.c: persistent_rice_adaptation_enabled_flag ? 2 * (av_log2(bit_depth - 10)) : 0; > > I can send you the file but i think this case is quite clear > > thx > > [...] > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 20/09/2024 01:54, James Almer wrote: > On 9/19/2024 9:34 PM, Michael Niedermayer wrote: >> On Thu, Sep 19, 2024 at 08:53:07PM -0300, James Almer wrote: >>> On 9/19/2024 7:56 PM, Michael Niedermayer wrote: >>>> Fixes: shift exponent 62 is too large for 32-bit type 'int' >>>> Fixes: >>>> 71020/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VVC_fuzzer-6444916325023744 >>>> >>>> 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_h266_syntax_template.c | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/libavcodec/cbs_h266_syntax_template.c >>>> b/libavcodec/cbs_h266_syntax_template.c >>>> index a8f5af04d02..1c111126563 100644 >>>> --- a/libavcodec/cbs_h266_syntax_template.c >>>> +++ b/libavcodec/cbs_h266_syntax_template.c >>>> @@ -1041,6 +1041,9 @@ static int >>>> FUNC(sps_range_extension)(CodedBitstreamContext *ctx, RWContext *rw, >>>> { >>>> int err; >>>> + if (current->sps_bitdepth_minus8 < 10) >>> >>> sps_bitdepth_minus8 can only be between 0 and 8, so this is basically >>> making >>> it abort on any and every sample with SPS range extension. >> >> + if (current->sps_bitdepth_minus8 < 10 - 8) > > Ok, this is different. > >> >> Its supposed to check this: >> "When BitDepth is less >> than or equal to 10, the value of sps_range_extension_flag shall be >> equal to 0." > > Should be "<= (10 - 8)" then, and LGTM. > LGTM, although nit: I think intent would be clearer and the code would correspond better with the standard if the check was moved to the parent function next to the flag itself.
On Fri, Sep 20, 2024 at 06:05:46PM +0100, Frank Plowman wrote: > On 20/09/2024 01:54, James Almer wrote: > > On 9/19/2024 9:34 PM, Michael Niedermayer wrote: > >> On Thu, Sep 19, 2024 at 08:53:07PM -0300, James Almer wrote: > >>> On 9/19/2024 7:56 PM, Michael Niedermayer wrote: > >>>> Fixes: shift exponent 62 is too large for 32-bit type 'int' > >>>> Fixes: > >>>> 71020/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VVC_fuzzer-6444916325023744 > >>>> > >>>> 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_h266_syntax_template.c | 3 +++ > >>>> 1 file changed, 3 insertions(+) > >>>> > >>>> diff --git a/libavcodec/cbs_h266_syntax_template.c > >>>> b/libavcodec/cbs_h266_syntax_template.c > >>>> index a8f5af04d02..1c111126563 100644 > >>>> --- a/libavcodec/cbs_h266_syntax_template.c > >>>> +++ b/libavcodec/cbs_h266_syntax_template.c > >>>> @@ -1041,6 +1041,9 @@ static int > >>>> FUNC(sps_range_extension)(CodedBitstreamContext *ctx, RWContext *rw, > >>>> { > >>>> int err; > >>>> + if (current->sps_bitdepth_minus8 < 10) > >>> > >>> sps_bitdepth_minus8 can only be between 0 and 8, so this is basically > >>> making > >>> it abort on any and every sample with SPS range extension. > >> > >> + if (current->sps_bitdepth_minus8 < 10 - 8) > > > > Ok, this is different. > > > >> > >> Its supposed to check this: > >> "When BitDepth is less > >> than or equal to 10, the value of sps_range_extension_flag shall be > >> equal to 0." > > > > Should be "<= (10 - 8)" then, and LGTM. will change > > > > LGTM, although nit: I think intent would be clearer and the code would > correspond better with the standard if the check was moved to the parent > function next to the flag itself. ok, will move it there thx [...]
diff --git a/libavcodec/cbs_h266_syntax_template.c b/libavcodec/cbs_h266_syntax_template.c index a8f5af04d02..1c111126563 100644 --- a/libavcodec/cbs_h266_syntax_template.c +++ b/libavcodec/cbs_h266_syntax_template.c @@ -1041,6 +1041,9 @@ static int FUNC(sps_range_extension)(CodedBitstreamContext *ctx, RWContext *rw, { int err; + if (current->sps_bitdepth_minus8 < 10) + return AVERROR_INVALIDDATA; + flag(sps_extended_precision_flag); if (current->sps_transform_skip_enabled_flag) flag(sps_ts_residual_coding_rice_present_in_sh_flag);
Fixes: shift exponent 62 is too large for 32-bit type 'int' Fixes: 71020/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_VVC_fuzzer-6444916325023744 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_h266_syntax_template.c | 3 +++ 1 file changed, 3 insertions(+)