| Message ID | 1386087086-3691-19-git-send-email-mst@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 3 December 2013 16:29, Michael S. Tsirkin <mst@redhat.com> 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. The commit message says it validates the row/col fields... > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/display/ssd0323.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/hw/display/ssd0323.c b/hw/display/ssd0323.c > index c3231c6..2c82598 100644 > --- a/hw/display/ssd0323.c > +++ b/hw/display/ssd0323.c > @@ -312,6 +312,9 @@ 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); > -- ...but the patch only checks the cmd_len ? thanks -- PMM
diff --git a/hw/display/ssd0323.c b/hw/display/ssd0323.c index c3231c6..2c82598 100644 --- a/hw/display/ssd0323.c +++ b/hw/display/ssd0323.c @@ -312,6 +312,9 @@ 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);
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 <mst@redhat.com> --- hw/display/ssd0323.c | 3 +++ 1 file changed, 3 insertions(+)