[FFmpeg-devel] FLAC crash on invalid data

[Michael Niedermayer]

On Thu, Oct 04, 2007 at 06:36:08AM -0400, Justin Ruggles wrote:
> Uoti Urpala wrote:
>> metadata_parse() does
>>             metadata_size = get_bits_long(&s->gb, 24);
>> then
>>                     for (i=0; i<metadata_size; i++)
>>                         skip_bits(&s->gb, 8);
>> with no sanity checks against read buffer size.
>
> Patch attached. Simpler than the last patch I submitted to fix the same 
> issue.

[...]
> Index: libavcodec/flac.c
> ===================================================================
> --- libavcodec/flac.c	(revision 10650)
> +++ libavcodec/flac.c	(working copy)
> @@ -190,7 +190,12 @@
>              av_log(s->avctx, AV_LOG_DEBUG,
>                     " metadata block: flag = %d, type = %d, size = %d\n",
>                     metadata_last, metadata_type, metadata_size);
> -            if (metadata_size) {
> +            if (metadata_size > 0) {

nonsense cosmetic change metadata_size cannot be <0


> +                int bits_left = s->gb.size_in_bits - get_bits_count(&s->gb);
> +                if(bits_left < metadata_size*8) {
> +                    skip_bits_long(&s->gb, bits_left);
> +                    break;
> +                }

dont silently ignore errors

the whole just needs a

if(invalid data)
    return appropriate error (unless that throws things into an inconsistant
    state but that doesnt seem to be the case here, also you could print an
    error message ...)

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071004/c19f5610/attachment.pgp>