[FFmpeg-devel] [PATCH 4/5] avcodec/alac: Check for bps of 0

James Almer jamrial at gmail.com
Sun Aug 25 18:06:30 EEST 2019


On 8/25/2019 4:32 AM, Michael Niedermayer wrote:
> On Fri, Aug 23, 2019 at 11:20:48AM -0300, James Almer wrote:
>> On 8/8/2019 8:23 PM, Michael Niedermayer wrote:
>>> Fixes: shift exponent 32 is too large for 32-bit type 'unsigned int'
>>> Fixes: 15764/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_ALAC_fuzzer-5102101203517440
>>>
>>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>>> ---
>>>  libavcodec/alac.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/alac.c b/libavcodec/alac.c
>>> index 6086e2caa8..1196925aa7 100644
>>> --- a/libavcodec/alac.c
>>> +++ b/libavcodec/alac.c
>>> @@ -250,7 +250,7 @@ static int decode_element(AVCodecContext *avctx, AVFrame *frame, int ch_index,
>>>  
>>>      alac->extra_bits = get_bits(&alac->gb, 2) << 3;
>>>      bps = alac->sample_size - alac->extra_bits + channels - 1;
>>> -    if (bps > 32U) {
>>> +    if (bps > 32 || bps < 1) {
>>>          avpriv_report_missing_feature(avctx, "bps %d", bps);
>>>          return AVERROR_PATCHWELCOME;
>>
>> bps 0 (or negative) is obviously a broken file, 
> 
> id say very likely a broken file, yes
> 
> 
>> so asking for a sample
>> is pointless. Just return invalid data in those cases, and leave this
>> check for > 32.
> 
> thats a few lines more code, for an error code and different/no message
> its a bit difficult to guess where people prefer the extra code to be
> correct and where they prefer somewhat incorrect solutions to minimize 
> fuzzer found bugfixes.

I don't know who this was aimed to, but afaik i don't ask or don't
intend to ask for "incorrect solutions" for fuzzer found issues.
I simply stated that asking for a sample in a situation where it's known
that the sample is simply a broken/corrupt one makes no sense and can
end up wasting a user's time for bothering to submit it. The extra lines
to properly abort in such cases are justified.

> 
> but yes, will post a new patch for this.
> 
> thx
> 
> 
> [...]
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> 



More information about the ffmpeg-devel mailing list