hw/display/qxl-render.c | 7 +++++++ hw/display/vmware_vga.c | 2 ++ ui/cursor.c | 8 +++++++- 3 files changed, 16 insertions(+), 1 deletion(-)
Prevent potential integer overflow by limiting 'width' and 'height' to
512x512. Also change 'datasize' type to size_t. Refer to security
advisory https://starlabs.sg/advisories/22-4206/ for more information.
Fixes: CVE-2021-4206
Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
---
v3:
- fix CVE id (CVE-2021-4206 instead of CVE-2022-4206)
hw/display/qxl-render.c | 7 +++++++
hw/display/vmware_vga.c | 2 ++
ui/cursor.c | 8 +++++++-
3 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index d28849b121..dc3c4edd05 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -247,6 +247,13 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor,
size_t size;
c = cursor_alloc(cursor->header.width, cursor->header.height);
+
+ if (!c) {
+ qxl_set_guest_bug(qxl, "%s: cursor %ux%u alloc error", __func__,
+ cursor->header.width, cursor->header.height);
+ goto fail;
+ }
+
c->hot_x = cursor->header.hot_spot_x;
c->hot_y = cursor->header.hot_spot_y;
switch (cursor->header.type) {
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 98c83474ad..45d06cbe25 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -515,6 +515,8 @@ static inline void vmsvga_cursor_define(struct vmsvga_state_s *s,
int i, pixels;
qc = cursor_alloc(c->width, c->height);
+ assert(qc != NULL);
+
qc->hot_x = c->hot_x;
qc->hot_y = c->hot_y;
switch (c->bpp) {
diff --git a/ui/cursor.c b/ui/cursor.c
index 1d62ddd4d0..835f0802f9 100644
--- a/ui/cursor.c
+++ b/ui/cursor.c
@@ -46,6 +46,8 @@ static QEMUCursor *cursor_parse_xpm(const char *xpm[])
/* parse pixel data */
c = cursor_alloc(width, height);
+ assert(c != NULL);
+
for (pixel = 0, y = 0; y < height; y++, line++) {
for (x = 0; x < height; x++, pixel++) {
idx = xpm[line][x];
@@ -91,7 +93,11 @@ QEMUCursor *cursor_builtin_left_ptr(void)
QEMUCursor *cursor_alloc(int width, int height)
{
QEMUCursor *c;
- int datasize = width * height * sizeof(uint32_t);
+ size_t datasize = width * height * sizeof(uint32_t);
+
+ if (width > 512 || height > 512) {
+ return NULL;
+ }
c = g_malloc0(sizeof(QEMUCursor) + datasize);
c->width = width;
--
2.35.1
On Thu, Apr 7, 2022 at 12:23 PM Mauro Matteo Cascella <mcascell@redhat.com> wrote: > Prevent potential integer overflow by limiting 'width' and 'height' to > 512x512. Also change 'datasize' type to size_t. Refer to security > advisory https://starlabs.sg/advisories/22-4206/ for more information. > > Fixes: CVE-2021-4206 > (the Starlabs advisory has 2022, I guess it's wrong then) Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > v3: > - fix CVE id (CVE-2021-4206 instead of CVE-2022-4206) > > hw/display/qxl-render.c | 7 +++++++ > hw/display/vmware_vga.c | 2 ++ > ui/cursor.c | 8 +++++++- > 3 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c > index d28849b121..dc3c4edd05 100644 > --- a/hw/display/qxl-render.c > +++ b/hw/display/qxl-render.c > @@ -247,6 +247,13 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, > QXLCursor *cursor, > size_t size; > > c = cursor_alloc(cursor->header.width, cursor->header.height); > + > + if (!c) { > + qxl_set_guest_bug(qxl, "%s: cursor %ux%u alloc error", __func__, > + cursor->header.width, cursor->header.height); > + goto fail; > + } > + > c->hot_x = cursor->header.hot_spot_x; > c->hot_y = cursor->header.hot_spot_y; > switch (cursor->header.type) { > diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c > index 98c83474ad..45d06cbe25 100644 > --- a/hw/display/vmware_vga.c > +++ b/hw/display/vmware_vga.c > @@ -515,6 +515,8 @@ static inline void vmsvga_cursor_define(struct > vmsvga_state_s *s, > int i, pixels; > > qc = cursor_alloc(c->width, c->height); > + assert(qc != NULL); > + > qc->hot_x = c->hot_x; > qc->hot_y = c->hot_y; > switch (c->bpp) { > diff --git a/ui/cursor.c b/ui/cursor.c > index 1d62ddd4d0..835f0802f9 100644 > --- a/ui/cursor.c > +++ b/ui/cursor.c > @@ -46,6 +46,8 @@ static QEMUCursor *cursor_parse_xpm(const char *xpm[]) > > /* parse pixel data */ > c = cursor_alloc(width, height); > + assert(c != NULL); > + > for (pixel = 0, y = 0; y < height; y++, line++) { > for (x = 0; x < height; x++, pixel++) { > idx = xpm[line][x]; > @@ -91,7 +93,11 @@ QEMUCursor *cursor_builtin_left_ptr(void) > QEMUCursor *cursor_alloc(int width, int height) > { > QEMUCursor *c; > - int datasize = width * height * sizeof(uint32_t); > + size_t datasize = width * height * sizeof(uint32_t); > + > + if (width > 512 || height > 512) { > + return NULL; > + } > > c = g_malloc0(sizeof(QEMUCursor) + datasize); > c->width = width; > -- > 2.35.1 > > > -- Marc-André Lureau
On Thu, 7 Apr 2022 at 10:21, Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > > > > On Thu, Apr 7, 2022 at 12:23 PM Mauro Matteo Cascella <mcascell@redhat.com> wrote: >> >> Prevent potential integer overflow by limiting 'width' and 'height' to >> 512x512. Also change 'datasize' type to size_t. Refer to security >> advisory https://starlabs.sg/advisories/22-4206/ for more information. >> >> Fixes: CVE-2021-4206 > > > (the Starlabs advisory has 2022, I guess it's wrong then) > >> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com> > > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Does this fix (or any of the other cursor-related stuff I've seen floating past) need to go into 7.0 ? (ie is it release-critical?) thanks -- PMM
On Thu, Apr 07, 2022 at 06:46:00PM +0100, Peter Maydell wrote: > On Thu, 7 Apr 2022 at 10:21, Marc-André Lureau > <marcandre.lureau@gmail.com> wrote: > > > > > > > > On Thu, Apr 7, 2022 at 12:23 PM Mauro Matteo Cascella <mcascell@redhat.com> wrote: > >> > >> Prevent potential integer overflow by limiting 'width' and 'height' to > >> 512x512. Also change 'datasize' type to size_t. Refer to security > >> advisory https://starlabs.sg/advisories/22-4206/ for more information. > >> > >> Fixes: CVE-2021-4206 > > > > > > (the Starlabs advisory has 2022, I guess it's wrong then) > > > >> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com> > > > > > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > Does this fix (or any of the other cursor-related stuff I've seen > floating past) need to go into 7.0 ? (ie is it release-critical?) Yes. The integer overflow can be triggered easily by guests. Hitting the double read race condition is harder but probably possible too. Pull request sent minutes ago. take care, Gerd
On Thu, Apr 7, 2022 at 11:17 AM Marc-André Lureau <marcandre.lureau@gmail.com> wrote: > > > > On Thu, Apr 7, 2022 at 12:23 PM Mauro Matteo Cascella <mcascell@redhat.com> wrote: >> >> Prevent potential integer overflow by limiting 'width' and 'height' to >> 512x512. Also change 'datasize' type to size_t. Refer to security >> advisory https://starlabs.sg/advisories/22-4206/ for more information. >> >> Fixes: CVE-2021-4206 > > > (the Starlabs advisory has 2022, I guess it's wrong then) Yep, that is wrong. I asked them to update the page. Thanks. >> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com> > > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > >> >> --- >> v3: >> - fix CVE id (CVE-2021-4206 instead of CVE-2022-4206) >> >> hw/display/qxl-render.c | 7 +++++++ >> hw/display/vmware_vga.c | 2 ++ >> ui/cursor.c | 8 +++++++- >> 3 files changed, 16 insertions(+), 1 deletion(-) >> >> diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c >> index d28849b121..dc3c4edd05 100644 >> --- a/hw/display/qxl-render.c >> +++ b/hw/display/qxl-render.c >> @@ -247,6 +247,13 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor, >> size_t size; >> >> c = cursor_alloc(cursor->header.width, cursor->header.height); >> + >> + if (!c) { >> + qxl_set_guest_bug(qxl, "%s: cursor %ux%u alloc error", __func__, >> + cursor->header.width, cursor->header.height); >> + goto fail; >> + } >> + >> c->hot_x = cursor->header.hot_spot_x; >> c->hot_y = cursor->header.hot_spot_y; >> switch (cursor->header.type) { >> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c >> index 98c83474ad..45d06cbe25 100644 >> --- a/hw/display/vmware_vga.c >> +++ b/hw/display/vmware_vga.c >> @@ -515,6 +515,8 @@ static inline void vmsvga_cursor_define(struct vmsvga_state_s *s, >> int i, pixels; >> >> qc = cursor_alloc(c->width, c->height); >> + assert(qc != NULL); >> + >> qc->hot_x = c->hot_x; >> qc->hot_y = c->hot_y; >> switch (c->bpp) { >> diff --git a/ui/cursor.c b/ui/cursor.c >> index 1d62ddd4d0..835f0802f9 100644 >> --- a/ui/cursor.c >> +++ b/ui/cursor.c >> @@ -46,6 +46,8 @@ static QEMUCursor *cursor_parse_xpm(const char *xpm[]) >> >> /* parse pixel data */ >> c = cursor_alloc(width, height); >> + assert(c != NULL); >> + >> for (pixel = 0, y = 0; y < height; y++, line++) { >> for (x = 0; x < height; x++, pixel++) { >> idx = xpm[line][x]; >> @@ -91,7 +93,11 @@ QEMUCursor *cursor_builtin_left_ptr(void) >> QEMUCursor *cursor_alloc(int width, int height) >> { >> QEMUCursor *c; >> - int datasize = width * height * sizeof(uint32_t); >> + size_t datasize = width * height * sizeof(uint32_t); >> + >> + if (width > 512 || height > 512) { >> + return NULL; >> + } >> >> c = g_malloc0(sizeof(QEMUCursor) + datasize); >> c->width = width; >> -- >> 2.35.1 >> >> > > > -- > Marc-André Lureau -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
© 2016 - 2024 Red Hat, Inc.