[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 21/30] ssd0323: fix buffer overun on invalid
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH v4 21/30] ssd0323: fix buffer overun on invalid state load |
Date: |
Mon, 31 Mar 2014 16:35:52 +0100 |
On 31 March 2014 15:17, Michael S. Tsirkin <address@hidden> wrote:
> CVE-2013-4538
>
> s->cmd_len used as index in ssd0323_transfer() to store 32-bit field.
> Possible this field might then be supplied by guest to overwrite a
> return addr somewhere. Same for row/col fields, which are indicies into
> framebuffer array.
>
> To fix validate after load.
>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
> hw/display/ssd0323.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/hw/display/ssd0323.c b/hw/display/ssd0323.c
> index 971152e..b520c69 100644
> --- a/hw/display/ssd0323.c
> +++ b/hw/display/ssd0323.c
> @@ -312,13 +312,22 @@ static int ssd0323_load(QEMUFile *f, void *opaque, int
> version_id)
> return -EINVAL;
>
> s->cmd_len = qemu_get_be32(f);
> + if (s->cmd_len < 0 || s->cmd_len > ARRAY_SIZE(s->cmd_data)) {
> + return -EINVAL;
> + }
> s->cmd = qemu_get_be32(f);
> for (i = 0; i < 8; i++)
> s->cmd_data[i] = qemu_get_be32(f);
> s->row = qemu_get_be32(f);
> + if (s->row < 0 || s->row >= 80 ) {
> + return -EINVAL;
> + }
> s->row_start = qemu_get_be32(f);
> s->row_end = qemu_get_be32(f);
> s->col = qemu_get_be32(f);
> + if (s->col < 0 || s->col >= 64 ) {
> + return -EINVAL;
> + }
> s->col_start = qemu_get_be32(f);
> s->col_end = qemu_get_be32(f);
> s->redraw = qemu_get_be32(f);
This isn't sufficient. You also need to validate that
the row/col_start/end are within bounds; otherwise the
guest can provoke an overrun by either setting the
_end field so large that the row++ increments just walk
off the end of the array, or by setting the _start
value to something bogus and then letting the "we hit
end of row" logic reset row to row_start.
thanks
-- PMM
- [Qemu-devel] [PATCH v4 14/30] stellaris_enet: avoid buffer overrun on incoming migration (part 2), (continued)
- [Qemu-devel] [PATCH v4 14/30] stellaris_enet: avoid buffer overrun on incoming migration (part 2), Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 15/30] stellaris_enet: avoid buffer orerrun on incoming migration (part 3), Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 16/30] virtio: avoid buffer overrun on incoming migration, Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 17/30] openpic: avoid buffer overrun on incoming migration, Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 18/30] virtio: validate num_sg when mapping, Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 19/30] pxa2xx: avoid buffer overrun on incoming migration, Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 21/30] ssd0323: fix buffer overun on invalid state load, Michael S. Tsirkin, 2014/03/31
- Re: [Qemu-devel] [PATCH v4 21/30] ssd0323: fix buffer overun on invalid state load,
Peter Maydell <=
- [Qemu-devel] [PATCH v4 20/30] ssi-sd: fix buffer overrun on invalid state load, Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 23/30] zaurus: fix buffer overrun on invalid state load, Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 22/30] tsc210x: fix buffer overrun on invalid state load, Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 24/30] usb: sanity check setup_index+setup_len in post_load, Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 25/30] virtio-scsi: fix buffer overrun on invalid state load, Michael S. Tsirkin, 2014/03/31
- [Qemu-devel] [PATCH v4 26/30] savevm: fix potential segfault on invalid state, Michael S. Tsirkin, 2014/03/31