[Spice-devel] [PATCH] quic: Fix "OVERRUN" caught by coverity

[Uri Lublin]

On 07/14/2014 11:14 AM, Fabiano Fidêncio wrote:
> Pre-incrementing the variable used as the array's index can make such
> index exceed the array's bounds. Although that (...)->rgb_state.melcstate
> has to be less than MELCSTATE to get inside the branch, it may be up to
> MELCSTATE after the pre-incrementing, which would result in an access to
> a position that is out bounds of the array sized MELCSTATE.

Hi,


I think it's safer to change the condition (MELCSTATES-1) than
changing the pre/post increment.
melcstate and melclen (and melcorder) should be kept in sync.

Thanks,
     Uri


> ---
>   common/quic.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/common/quic.c b/common/quic.c
> index c10e3c4..ea6bd71 100644
> --- a/common/quic.c
> +++ b/common/quic.c
> @@ -579,7 +579,7 @@ static void encode_run(Encoder *encoder, unsigned int runlen) //todo: try use en
>           hits++;
>           runlen -= encoder->rgb_state.melcorder;
>           if (encoder->rgb_state.melcstate < MELCSTATES) {
> -            encoder->rgb_state.melclen = J[++encoder->rgb_state.melcstate];
> +            encoder->rgb_state.melclen = J[encoder->rgb_state.melcstate++];
>               encoder->rgb_state.melcorder = (1L << encoder->rgb_state.melclen);
>           }
>       }
> @@ -611,7 +611,7 @@ static void encode_channel_run(Encoder *encoder, Channel *channel, unsigned int
>           hits++;
>           runlen -= channel->state.melcorder;
>           if (channel->state.melcstate < MELCSTATES) {
> -            channel->state.melclen = J[++channel->state.melcstate];
> +            channel->state.melclen = J[channel->state.melcstate++];
>               channel->state.melcorder = (1L << channel->state.melclen);
>           }
>       }
> @@ -648,7 +648,7 @@ static int decode_run(Encoder *encoder)
>               runlen += encoder->rgb_state.melcorder;
>   
>               if (encoder->rgb_state.melcstate < MELCSTATES) {
> -                encoder->rgb_state.melclen = J[++encoder->rgb_state.melcstate];
> +                encoder->rgb_state.melclen = J[encoder->rgb_state.melcstate++];
>                   encoder->rgb_state.melcorder = (1U << encoder->rgb_state.melclen);
>               }
>           }
> @@ -689,7 +689,7 @@ static int decode_channel_run(Encoder *encoder, Channel *channel)
>               runlen += channel->state.melcorder;
>   
>               if (channel->state.melcstate < MELCSTATES) {
> -                channel->state.melclen = J[++channel->state.melcstate];
> +                channel->state.melclen = J[channel->state.melcstate++];
>                   channel->state.melcorder = (1U << channel->state.melclen);
>               }
>           }