[FFmpeg-devel] [PATCH] FLAC decoder segfault reading metadata

[Justin Ruggles]

Michael Niedermayer wrote:
> Hi
> 
> On Tue, Oct 02, 2007 at 08:45:40PM -0400, Justin Ruggles wrote:
> [...]
>>>> +            if(s->bps < 4) {
>>>> +                av_log(s->avctx, AV_LOG_DEBUG, "invalid bits-per-sample. must be >= 4.\n");
>>>> +                return 0;
>>>> +            }
>>>>              allocate_buffers(s);
>>>> +        }
>>> patch rejected, you cant just skip reallocating
>>> the buffers, the return of this function is not checked nothing will
>>> stop the randomly sized buffers from being used
>> If this function returns an error (for some odd reason 0 is error...) 
>> then no decoding will be attempted.  
> 
> NO
> 
> flac.c:----------
>         } else {
>             metadata_parse(s);
>         }
>     }
> 
>     return 0;
> }
> -----------------
> the return value is NOT checked!
> 
> 
> 
>> It doesn't make sense to resize the
>> buffers when the information you're basing the new sizes on is incorrect.
> 
> the buffers MUST always be as large as the buf sizes you store in the struct
> and use to prevent overflows

You're right.  And I agree.  I had missed the fact that the decoder only 
stops decoding due to incorrect header information if there is actually 
something there to check.

In the end, I want it to function sanely.  The decoder should always get 
the proper information from extradata at initialization and resize the 
buffers accordingly, otherwise initialization should fail and decoding 
should not proceed.

-Justin