[FFmpeg-devel] [libav-devel] [PATCH] riffdec: error out on negative bit rate

Andreas Cadhalpun andreas.cadhalpun at googlemail.com
Sat Jul 11 17:15:20 CEST 2015


On 11.07.2015 12:30, Luca Barbato wrote:
> On 11/07/15 11:58, Kostya Shishkov wrote:
>> On Sat, Jul 11, 2015 at 11:35:44AM +0200, Luca Barbato wrote:
>>> On 11/07/15 00:39, Andreas Cadhalpun wrote:
>>>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
>>>> ---
>>>>  libavformat/riffdec.c | 5 +++++
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/libavformat/riffdec.c b/libavformat/riffdec.c
>>>> index eebd8ed..be55699 100644
>>>> --- a/libavformat/riffdec.c
>>>> +++ b/libavformat/riffdec.c
>>>> @@ -106,6 +106,11 @@ int ff_get_wav_header(AVIOContext *pb, AVCodecContext *codec, int size, int big_
>>>>          codec->bit_rate    = avio_rb32(pb) * 8;
>>>>          codec->block_align = avio_rb16(pb);
>>>>      }
>>>> +    if (codec->bit_rate < 0) {
>>>> +        av_log(NULL, AV_LOG_ERROR,
>>>> +               "Invalid bit rate: %d\n", codec->bit_rate);
>>>> +        return AVERROR_INVALIDDATA;
>>>> +    }
>>>
>>> I'm quite sure some codecs do not have a fixed bit_rate while some other
>>> (like the g726 below) do use it.
>>>
>>> So I'd expect this change to break on some semi-valid files and prevent
>>> something horrid in other.
>>
>> I know how to prevent it in much easier way - RTFM, see that byte rate in
>> WAVEFORMAT is an unsigned number and get back to sleep.
> 
> int bit_rate = avio_rl32(pb) * 8;
> 
> Anything larger than INT_MAX (and possibly much less) could be safely
> rejected as invalid.

Rejecting negative values has practically the same effect, even though
theoretically signed integer overflow is not well-defined.

> The question is if reject is as invalid by erroring out or just set to 0
> as in unknown.

Providing an explode mode is the common compromise.

Best regards,
Andreas


More information about the ffmpeg-devel mailing list