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

[Alex Converse]

On Wed, Dec 29, 2010 at 7:01 AM, Reimar D?ffinger
<Reimar.Doeffinger at gmx.de> wrote:
>
> 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.

The codereview URL is http://codereview.chromium.org/5964011
The bug URL is broken.
That is all the information the allmighty GOOG supplied.