On 31 March 2014 15:17, Michael S. Tsirkin <m...@redhat.com> wrote:
> 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.
>
> Cc: Andreas Färber <afaer...@suse.de>
> Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> ---
> hw/input/tsc210x.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/hw/input/tsc210x.c b/hw/input/tsc210x.c
> index 485c9e5..65a0d08 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)) {
> + return -EINVAL;
> + }
Why no check for negative values? Also, shouldn't
this be >=, like the checks below?
> s->nextfunction = qemu_get_byte(f);
> + if (s->nextfunction < 0 || s->nextfunction >= ARRAY_SIZE(mode_regs)) {
> + return -EINVAL;
> + }
> s->precision = qemu_get_byte(f);
> + if (s->precision < 0 || s->precision >= ARRAY_SIZE(resolution)) {
> + return -EINVAL;
> + }
> s->nextprecision = qemu_get_byte(f);
> + if (s->nextprecision < 0 || 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);
> --
> MST
>
thanks
-- PMM
