[FFmpeg-devel] [PATCH] Escape 124 (RPL) decoder rev3

[Eli Friedman]

On Sat, Mar 29, 2008 at 11:47 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>  > +            if (i == 0) {
>  > +                // This is the most basic codebook: pow(2,depth) entries for
>  > +                // a depth-length key
>  > +                cb_depth = get_bits(&gb, 4);
>  > +                cb_size = 1 << cb_depth;
>  > +                cb_alloc_size = cb_size;
>  > +            } else if (i == 1) {
>  > +                // This codebook varies per superblock
>  > +                // FIXME: I don't think this handles integer overflow
>  > +                // properly
>  > +                cb_depth = get_bits(&gb, 4);
>  > +                cb_size = s->num_superblocks << cb_depth;
>  > +                cb_alloc_size = cb_size;
>  > +            } else {
>  > +                // This codebook can be cut off at places other than
>  > +                // powers of 2, leaving some of the entries undefined.
>  > +                cb_size = get_bits_long(&gb, 20);
>  > +                cb_depth = av_log2(cb_size - 1) + 1;
>  > +                cb_alloc_size = 1 << cb_depth;
>  > +            }
>
>  What is the use of having cb_alloc_size > cb_size ?

So that the code doesn't crash on invalid bitstreams.

>  > +        if (can_safely_read(&gb, 1) && !get_bits1(&gb)) {
>
>  i think the can_safely_read is unneeded here.

Possibly, although it's cutting it a bit close; I count 59 bits of
potential overread without that check.

-Eli