[FFmpeg-devel] [PATCH] use av_mallocz() in vorbis_comment()

[Justin Ruggles]

M?ns Rullg?rd wrote:
> Justin Ruggles <justin.ruggles at gmail.com> writes:
> 
>> Benoit Fouet wrote:
>>> On 02/12/2009 03:45 AM, Justin Ruggles wrote:
>>>> M?ns Rullg?rd wrote:
>>>>   
>>>>> Justin Ruggles <justin.ruggles at gmail.com> writes:
>>>>>
>>>>>     
>>>>>> Hi,
>>>>>>
>>>>>> This patch avoids allocating memory on the stack based on decoded stream
>>>>>> values which can be up to 32-bit.  Mans has pointed out that the current
>>>>>> version is not a security risk, it would just crash with SIGSEGV for
>>>>>> really large metadata.  This patch skips the single metadata tag if
>>>>>> allocation fails and continues try to the next tag.
>>>>>>
>>>>>> Thanks,
>>>>>> Justin
>>>>>>
>>>>>>
>>>>>> Index: libavformat/oggparsevorbis.c
>>>>>> ===================================================================
>>>>>> --- libavformat/oggparsevorbis.c	(revision 17145)
>>>>>> +++ libavformat/oggparsevorbis.c	(working copy)
>>>>>> @@ -71,15 +71,21 @@
>>>>>>          v++;
>>>>>>  
>>>>>>          if (tl && vl) {
>>>>>> -            char tt[tl + 1];
>>>>>> -            char ct[vl + 1];
>>>>>> +            char *tt, *ct;
>>>>>>  
>>>>>> +            tt = av_mallocz(tl + 1);
>>>>>> +            ct = av_mallocz(vl + 1);
>>>>>>       
>>>>> Why mallocz?  It's being written again immediately below.
>>>>>     
>>>> No particular reason. New patch attached.
>>>>
>>>>   
>>> isn't this patch missing some av_freep ?
>> oops... new patch attached.
>>
>>
>> Index: libavformat/oggparsevorbis.c
>> ===================================================================
>> --- libavformat/oggparsevorbis.c	(revision 17174)
>> +++ libavformat/oggparsevorbis.c	(working copy)
>> @@ -71,9 +71,17 @@
>>          v++;
>>  
>>          if (tl && vl) {
>> -            char tt[tl + 1];
>> -            char ct[vl + 1];
>> +            char *tt, *ct;
>>  
>> +            tt = av_malloc(tl + 1);
>> +            ct = av_malloc(vl + 1);
>> +            if (!tt || !ct) {
>> +                av_freep(&tt);
>> +                av_freep(&ct);
>> +                av_log(as, AV_LOG_WARNING, "out-of-memory error. skipping VorbisComment tag.\n");
>> +                continue;
>> +            }
>> +
>>              for (j = 0; j < tl; j++)
>>                  tt[j] = toupper(t[j]);
>>              tt[tl] = 0;
>> @@ -82,6 +90,9 @@
>>              ct[vl] = 0;
>>  
>>              av_metadata_set(&as->metadata, tt, ct);
>> +
>> +            av_freep(&tt);
>> +            av_freep(&ct);
>>          }
>>      }
>>  
> 
> OK

applied.