[FFmpeg-devel] [PATCH 7/7] atrac3plus: Prevent array out-of-bounds

[Timothy Gu]

On Fri Feb 06 2015 at 2:58:19 AM wm4 <nfxjfg at googlemail.com> wrote:

> On Fri, 06 Feb 2015 07:32:56 +0000
> Timothy Gu <timothygu99 at gmail.com> wrote:
>
> > On Thu Feb 05 2015 at 11:07:01 PM Timothy Gu <timothygu99 at gmail.com>
> wrote:
> >
> > > (num_quant_units - 1) is later used as an index to
> atrac3p_qu_to_subband,
> > > which only has 32 elements (i.e. maximum of num_quant_units is 32).
> > > ---
> > >  libavcodec/atrac3plus.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> >
> > Note that this doesn't actually fix any problem else than a GCC warning.
> >
> >
> > >
> > > diff --git a/libavcodec/atrac3plus.c b/libavcodec/atrac3plus.c
> > > index 575a493..b215b02 100644
> > > --- a/libavcodec/atrac3plus.c
> > > +++ b/libavcodec/atrac3plus.c
> > > @@ -1768,7 +1768,7 @@ int ff_atrac3p_decode_channel_unit(GetBitContext
> > > *gb, Atrac3pChanUnitCtx *ctx,
> > >
> > >      /* parse sound header */
> > >      ctx->num_quant_units = get_bits(gb, 5) + 1;
> > >
> >
> > num_quant_units can only be <= (2^5 - 1) + 1, which is <= 32.
> >
> > This just makes it easier for GCC to see that.
> >
> >
> > > -    if (ctx->num_quant_units > 28 && ctx->num_quant_units < 32) {
> > > +    if (ctx->num_quant_units > 28 && ctx->num_quant_units != 32) {
> > >          av_log(avctx, AV_LOG_ERROR,
> > >                 "Invalid number of quantization units: %d!\n",
> > >                 ctx->num_quant_units);
> > >
> >
>
> Maybe I'm missing something, but how does that
> justify != instead of <= ?
>

Before the compiler thinks:

"OK, the variable can be in the range [0, 28] U [32, INT_MAX]"

But because it is used as a array index, it can only be safely used < 32,
and hence the warning.

Now the compiler thinks:

"The variable can only be in the range [0, 28] U {32}."

<= won't work because it still makes 33 possible as far as the compiler is
concerned.

Timothy