[FFmpeg-devel] [PATCH] Fix a couple of errors with bad Vorbis headers

[Reimar Döffinger]

On Tue, Dec 28, 2010 at 08:19:03PM -0800, Alex Converse wrote:
> Patch by Chris Evans/Google Chrome
> 
> Chrome review URL:
> http://codereview.chromiumcodereview.org/5964011<http://codereview.chromium.org/5964011>
> Chrome bug URL:
> http://code.google.com/p/chromium/issues/detail?id=68815(Broken?)

> diff -wurp -N orig/libavcodec/vorbis_dec.c ffmpeg-mt/libavcodec/vorbis_dec.c
> --- orig/libavcodec/vorbis_dec.c	2010-12-27 11:16:48.320721968 -0800
> +++ ffmpeg-mt/libavcodec/vorbis_dec.c	2010-12-27 11:55:33.241708823 -0800
> @@ -536,6 +536,12 @@ static int vorbis_parse_setup_hdr_floors
>              rangebits = get_bits(gb, 4);
>              floor_setup->data.t1.list[0].x = 0;
>              floor_setup->data.t1.list[1].x = (1 << rangebits);
> +            if (floor_setup->data.t1.list[1].x > vc->blocksize[1] / 2) {
> +                av_log(vc->avccontext, AV_LOG_ERROR,
> +                       "Floor value is too large for blocksize: %d (%d)\n",
> +                       floor_setup->data.t1.list[1].x, vc->blocksize[1] / 2);
> +                return -1;
> +            }
>  
>              for (j = 0; j < floor_setup->data.t1.partitions; ++j) {
>                  for (k = 0; k < floor_setup->data.t1.class_dimensions[floor_setup->data.t1.partition_class[j]]; ++k, ++floor1_values) {

You leave the invalid value in there, this is quite risky IMO.

> @@ -653,7 +659,7 @@ static int vorbis_parse_setup_hdr_residu
>          res_setup->partition_size = get_bits(gb, 24) + 1;
>          /* Validations to prevent a buffer overflow later. */
>          if (res_setup->begin>res_setup->end ||
> -            res_setup->end > vc->avccontext->channels * vc->blocksize[1] / (res_setup->type == 2 ? 1 : 2) ||
> +            res_setup->end > vc->avccontext->channels * vc->blocksize[1] / 2 ||

I'd say this change needs at least some explanation, neither of the URLs
you gave work for me.