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

[Måns Rullgård]

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

-- 
M?ns Rullg?rd
mans at mansr.com