diff mbox

[v4,22/30] tsc210x: fix buffer overrun on invalid state load

Message ID 1396275242-10810-23-git-send-email-mst@redhat.com
State New
Headers show

Commit Message

Michael S. Tsirkin March 31, 2014, 2:17 p.m. UTC
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 <afaerber@suse.de>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/input/tsc210x.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Peter Maydell March 31, 2014, 3:39 p.m. UTC | #1
On 31 March 2014 15:17, Michael S. Tsirkin <mst@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 <afaerber@suse.de>
> Signed-off-by: Michael S. Tsirkin <mst@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
diff mbox

Patch

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;
+    }
     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);