Re: [FFmpeg-devel] [PATCH 1/4] avformat/iamf_parse: Check extradata size

[James Almer <jamrial-at-gmail.com@ffmpeg.org>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2011 bytes --]

On 6/23/2025 9:44 AM, Michael Niedermayer wrote:
> On Fri, Jun 20, 2025 at 12:28:13AM +0200, Andreas Rheinhardt wrote:
>> Michael Niedermayer:
>>> Fixes: Assertion n>=0 && n<=32 failed at ./libavcodec/get_bits.h:406
>>> Fixes: 398527871/clusterfuzz-testcase-minimized-ffmpeg_dem_IAMF_fuzzer-6602025714647040
>>>
>>> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>>   libavformat/iamf_parse.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/libavformat/iamf_parse.c b/libavformat/iamf_parse.c
>>> index 71497876ac3..330e01733dd 100644
>>> --- a/libavformat/iamf_parse.c
>>> +++ b/libavformat/iamf_parse.c
>>> @@ -305,6 +305,8 @@ static int update_extradata(AVCodecParameters *codecpar)
>>>           skip_bits(&gb, 4);
>>>           put_bits(&pb, 4, codecpar->ch_layout.nb_channels); // set channel config
>>>           ret = put_bits_left(&pb);
>>> +        if (ret < 0)
>>> +            return AVERROR_INVALIDDATA;
>>>           while (ret >= 32) {
>>>              put_bits32(&pb, get_bits_long(&gb, 32));
>>>              ret -= 32;
>>
>> There is only one way for put_bits_left() to return a negative value: If
>> there is more data in the internal buffer than can be written out. And
>> this scenario is already a violation of the PutBit API. Given that the
>> size of the internal buffer depends upon the arch, it could be that one
>> would have already hit an assert in case one is not using x64. In other
>> words, your check is too late.
> 
> the patches puprose was mainly to show that
> 3f9420132441345b7ccd57001f230bb98f655696
> was insufficient to fix 398527871
> 
> I do not expect my patch would be the correct solution even if the
> check is done earlier. IAMF is cursed

Does increasing buf from 6 bytes to 8 or more fix it? I see putbits may 
do an AV_W*64(), so six bytes sounds like it was never safe.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".