Re: [Qemu-devel] [PATCH 19/23] tsc210x: fix buffer overrun on invalid state load

[Andreas Färber]

Am 03.12.2013 17:29, schrieb Michael S. Tsirkin:
> CVE-2013-4539
> 
> s->precision, nextprecision, function and nextfunction
> come from wire and are used
> as idx into resolution[] in TSC_CUT_RESOLUTION.
> 
> Validate after load to avoid buffer overrun.
> 
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
>  hw/input/tsc210x.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/input/tsc210x.c b/hw/input/tsc210x.c
> index 485c9e5..c7513c7 100644
> --- a/hw/input/tsc210x.c
> +++ b/hw/input/tsc210x.c
> @@ -1070,9 +1070,21 @@ static int tsc210x_load(QEMUFile *f, void *opaque, int 
> version_id)
>      s->enabled = qemu_get_byte(f);
>      s->host_mode = qemu_get_byte(f);
>      s->function = qemu_get_byte(f);
> +    if (s->function > ARRAY_SIZE(mode_regs)) {

Don't these need to be >= according to the commit message?

Regards,
Andreas

> +        return -EINVAL;
> +    }
>      s->nextfunction = qemu_get_byte(f);
> +    if (s->nextfunction > ARRAY_SIZE(mode_regs)) {
> +        return -EINVAL;
> +    }
>      s->precision = qemu_get_byte(f);
> +    if (s->precision > ARRAY_SIZE(resolution)) {
> +        return -EINVAL;
> +    }
>      s->nextprecision = qemu_get_byte(f);
> +    if (s->nextprecision > ARRAY_SIZE(resolution)) {
> +        return -EINVAL;
> +    }
>      s->filter = qemu_get_byte(f);
>      s->pin_func = qemu_get_byte(f);
>      s->ref = qemu_get_byte(f);
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg