[FFmpeg-cvslog] r19991 - trunk/libavcodec/vorbis_dec.c

[Reimar Döffinger]

On Wed, Sep 23, 2009 at 02:17:54PM +0200, michael wrote:
> Author: michael
> Date: Wed Sep 23 14:17:54 2009
> New Revision: 19991
> 
> Log:
> Check masterbook index and subclass book index.
> 14_floor_masterbook_index.patch by chrome
> 
> Modified:
>    trunk/libavcodec/vorbis_dec.c
> 
> Modified: trunk/libavcodec/vorbis_dec.c
> ==============================================================================
> --- trunk/libavcodec/vorbis_dec.c	Wed Sep 23 14:09:33 2009	(r19990)
> +++ trunk/libavcodec/vorbis_dec.c	Wed Sep 23 14:17:54 2009	(r19991)
> @@ -487,13 +487,23 @@ static int vorbis_parse_setup_hdr_floors
>                  AV_DEBUG(" %d floor %d class dim: %d subclasses %d \n", i, j, floor_setup->data.t1.class_dimensions[j], floor_setup->data.t1.class_subclasses[j]);
>  
>                  if (floor_setup->data.t1.class_subclasses[j]) {
> -                    floor_setup->data.t1.class_masterbook[j]=get_bits(gb, 8);
> +                    int bits=get_bits(gb, 8);
> +                    if (bits>=vc->codebook_count) {
> +                        av_log(vc->avccontext, AV_LOG_ERROR, "Masterbook index %d is out of range.\n", bits);
> +                        return 1;
> +                    }
> +                    floor_setup->data.t1.class_masterbook[j]=bits;
>  
>                      AV_DEBUG("   masterbook: %d \n", floor_setup->data.t1.class_masterbook[j]);
>                  }
>  
>                  for(k=0;k<(1<<floor_setup->data.t1.class_subclasses[j]);++k) {
> -                    floor_setup->data.t1.subclass_books[j][k]=(int16_t)get_bits(gb, 8)-1;
> +                    int16_t bits=get_bits(gb, 8)-1;
> +                    if (bits!=-1 && bits>=vc->codebook_count) {
> +                        av_log(vc->avccontext, AV_LOG_ERROR, "Subclass book index %d is out of range.\n", bits);
> +                        return 1;
> +                    }
> +                    floor_setup->data.t1.subclass_books[j][k]=bits;
>  
>                      AV_DEBUG("    book %d. : %d \n", k, floor_setup->data.t1.subclass_books[j][k]);
>                  }

I feel compelled that these checks, are inconsistent with the other
checks due to using that bits intermediate variable (in particular these
also couldn't use the macro I proposed because of that)...
I also think there is some inconsistency as in whether return 1 or
return -1 is use...