On 26/09/18 22:50, Carl Eugen Hoyos wrote:
> 2018-09-25 0:16 GMT+02:00, Mark Thompson <[email protected]>:
>> ffmpeg | branch: master | Mark Thompson <[email protected]> | Mon Sep 24 
>> 22:45:50
>> 2018 +0100| [321294adb788b5e143fcec776cdf1daf79ed921c] | committer: Mark
>> Thompson
>>
>> h264_metadata: Avoid integer overflow in bitrate
>>
>> Fixes CID #1439664.
>>
>>> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=321294adb788b5e143fcec776cdf1daf79ed921c
>> ---
>>
>>  libavcodec/h264_metadata_bsf.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavcodec/h264_metadata_bsf.c b/libavcodec/h264_metadata_bsf.c
>> index 991fcfa537..bf37528234 100644
>> --- a/libavcodec/h264_metadata_bsf.c
>> +++ b/libavcodec/h264_metadata_bsf.c
>> @@ -226,10 +226,10 @@ static int h264_metadata_update_sps(AVBSFContext *bsf,
>>
>>              if (sps->vui.nal_hrd_parameters_present_flag) {
>>                  bit_rate =
>> (sps->vui.nal_hrd_parameters.bit_rate_value_minus1[0] + 1) *
>> -                     (1 << (sps->vui.nal_hrd_parameters.bit_rate_scale +
>> 6));
> 
> How can this overflow?

bit_rate_value_minus1 can be up to UINT32_MAX - 1 and bit_rate_scale can be up 
to 15, so the product can get to just under 2^53.  While that isn't valid for 
any level, the fields come from untrusted user input.

>> +                    (INT64_C(1) <<
>> (sps->vui.nal_hrd_parameters.bit_rate_scale + 6));
>>              } else if (sps->vui.vcl_hrd_parameters_present_flag) {
>>                  bit_rate =
>> (sps->vui.vcl_hrd_parameters.bit_rate_value_minus1[0] + 1) *
>> -                     (1 << (sps->vui.vcl_hrd_parameters.bit_rate_scale +
>> 6));
> 
> Same here: Isn't the maximum (15+6) ?

While the first shift can't overflow, the whole expression can as noted above.  
Coverity was complaining specifically about the shift rather than the whole 
expression, so it seemed sensible to put the int64_t cast here.

>> +                    (INT64_C(1) <<
>> (sps->vui.vcl_hrd_parameters.bit_rate_scale + 6));
>>                  // Adjust for VCL vs. NAL limits.
>>                  bit_rate = bit_rate * 6 / 5;
>>              } else {

Thanks,

- Mark
_______________________________________________
ffmpeg-devel mailing list
[email protected]
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to