include/ui/console.h | 4 ++-- ui/cursor.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
The cursor_alloc function still accepts a signed integer for both the cursor
width and height. A specially crafted negative width/height could make datasize
wrap around and cause the next allocation to be 0, potentially leading to a
heap buffer overflow. Modify QEMUCursor struct and cursor_alloc prototype to
accept unsigned ints.
Fixes: CVE-2023-1601
Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc (CVE-2021-4206)")
Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
Reported-by: Jacek Halon <jacek.halon@gmail.com>
---
include/ui/console.h | 4 ++--
ui/cursor.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/ui/console.h b/include/ui/console.h
index 2a8fab091f..92a4d90a1b 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -144,13 +144,13 @@ typedef struct QemuUIInfo {
/* cursor data format is 32bit RGBA */
typedef struct QEMUCursor {
- int width, height;
+ uint32_t width, height;
int hot_x, hot_y;
int refcount;
uint32_t data[];
} QEMUCursor;
-QEMUCursor *cursor_alloc(int width, int height);
+QEMUCursor *cursor_alloc(uint32_t width, uint32_t height);
QEMUCursor *cursor_ref(QEMUCursor *c);
void cursor_unref(QEMUCursor *c);
QEMUCursor *cursor_builtin_hidden(void);
diff --git a/ui/cursor.c b/ui/cursor.c
index 6fe67990e2..b5fcb64839 100644
--- a/ui/cursor.c
+++ b/ui/cursor.c
@@ -90,7 +90,7 @@ QEMUCursor *cursor_builtin_left_ptr(void)
return cursor_parse_xpm(cursor_left_ptr_xpm);
}
-QEMUCursor *cursor_alloc(int width, int height)
+QEMUCursor *cursor_alloc(uint32_t width, uint32_t height)
{
QEMUCursor *c;
size_t datasize = width * height * sizeof(uint32_t);
--
2.40.1On Mon, May 08, 2023 at 04:18:13PM +0200, Mauro Matteo Cascella wrote:
> The cursor_alloc function still accepts a signed integer for both the cursor
> width and height. A specially crafted negative width/height could make datasize
> wrap around and cause the next allocation to be 0, potentially leading to a
> heap buffer overflow. Modify QEMUCursor struct and cursor_alloc prototype to
> accept unsigned ints.
>
I concur with Marc-Andre that there is no code path that can
actually trigger an overflow:
hw/display/ati.c: s->cursor = cursor_alloc(64, 64);
hw/display/vhost-user-gpu.c: s->current_cursor = cursor_alloc(64, 64);
hw/display/virtio-gpu.c: s->current_cursor = cursor_alloc(64, 64);
Not exploitable as fixed size
hw/display/qxl-render.c: c = cursor_alloc(cursor->header.width, cursor->header.height);
Cursor header defined as:
typedef struct SPICE_ATTR_PACKED QXLCursorHeader {
uint64_t unique;
uint16_t type;
uint16_t width;
uint16_t height;
uint16_t hot_spot_x;
uint16_t hot_spot_y;
} QXLCursorHeader;
So no negative values can be passed to cursor_alloc()
hw/display/vmware_vga.c: qc = cursor_alloc(c->width, c->height);
Where 'c' is defined as:
struct vmsvga_cursor_definition_s {
uint32_t width;
uint32_t height;
int id;
uint32_t bpp;
int hot_x;
int hot_y;
uint32_t mask[1024];
uint32_t image[4096];
};
and is also already bounds checked:
if (cursor.width > 256
|| cursor.height > 256
|| cursor.bpp > 32
|| SVGA_BITMAP_SIZE(x, y) > ARRAY_SIZE(cursor.mask)
|| SVGA_PIXMAP_SIZE(x, y, cursor.bpp)
> ARRAY_SIZE(cursor.image)) {
goto badcmd;
}
> Fixes: CVE-2023-1601
> Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc (CVE-2021-4206)")
Given there is no possible codepath that can overflow, CVE-2023-1601
looks invalid to me. It should be clsoed as not-a-bug and these two
Fixes lines removed.
> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> Reported-by: Jacek Halon <jacek.halon@gmail.com>
> ---
> include/ui/console.h | 4 ++--
> ui/cursor.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
Even though it isn't fixing a bug, the change itself still makes
sense, because there's no reason a negative width/height is ever
appropriate. This protects us against accidentally introducing
future bugs, so with the two CVE Fixes lines removed:
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 2a8fab091f..92a4d90a1b 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -144,13 +144,13 @@ typedef struct QemuUIInfo {
>
> /* cursor data format is 32bit RGBA */
> typedef struct QEMUCursor {
> - int width, height;
> + uint32_t width, height;
> int hot_x, hot_y;
> int refcount;
> uint32_t data[];
> } QEMUCursor;
>
> -QEMUCursor *cursor_alloc(int width, int height);
> +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height);
> QEMUCursor *cursor_ref(QEMUCursor *c);
> void cursor_unref(QEMUCursor *c);
> QEMUCursor *cursor_builtin_hidden(void);
> diff --git a/ui/cursor.c b/ui/cursor.c
> index 6fe67990e2..b5fcb64839 100644
> --- a/ui/cursor.c
> +++ b/ui/cursor.c
> @@ -90,7 +90,7 @@ QEMUCursor *cursor_builtin_left_ptr(void)
> return cursor_parse_xpm(cursor_left_ptr_xpm);
> }
>
> -QEMUCursor *cursor_alloc(int width, int height)
> +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height)
> {
> QEMUCursor *c;
> size_t datasize = width * height * sizeof(uint32_t);
> --
> 2.40.1
>
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, May 23, 2023 at 10:16 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Mon, May 08, 2023 at 04:18:13PM +0200, Mauro Matteo Cascella wrote:
> > The cursor_alloc function still accepts a signed integer for both the cursor
> > width and height. A specially crafted negative width/height could make datasize
> > wrap around and cause the next allocation to be 0, potentially leading to a
> > heap buffer overflow. Modify QEMUCursor struct and cursor_alloc prototype to
> > accept unsigned ints.
> >
> I concur with Marc-Andre that there is no code path that can
> actually trigger an overflow:
>
>
> hw/display/ati.c: s->cursor = cursor_alloc(64, 64);
> hw/display/vhost-user-gpu.c: s->current_cursor = cursor_alloc(64, 64);
> hw/display/virtio-gpu.c: s->current_cursor = cursor_alloc(64, 64);
>
> Not exploitable as fixed size
>
> hw/display/qxl-render.c: c = cursor_alloc(cursor->header.width, cursor->header.height);
>
> Cursor header defined as:
>
> typedef struct SPICE_ATTR_PACKED QXLCursorHeader {
> uint64_t unique;
> uint16_t type;
> uint16_t width;
> uint16_t height;
> uint16_t hot_spot_x;
> uint16_t hot_spot_y;
> } QXLCursorHeader;
>
> So no negative values can be passed to cursor_alloc()
>
>
> hw/display/vmware_vga.c: qc = cursor_alloc(c->width, c->height);
>
> Where 'c' is defined as:
>
> struct vmsvga_cursor_definition_s {
> uint32_t width;
> uint32_t height;
> int id;
> uint32_t bpp;
> int hot_x;
> int hot_y;
> uint32_t mask[1024];
> uint32_t image[4096];
> };
>
> and is also already bounds checked:
>
> if (cursor.width > 256
> || cursor.height > 256
> || cursor.bpp > 32
> || SVGA_BITMAP_SIZE(x, y) > ARRAY_SIZE(cursor.mask)
> || SVGA_PIXMAP_SIZE(x, y, cursor.bpp)
> > ARRAY_SIZE(cursor.image)) {
> goto badcmd;
> }
>
> > Fixes: CVE-2023-1601
> > Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc (CVE-2021-4206)")
>
> Given there is no possible codepath that can overflow, CVE-2023-1601
> looks invalid to me. It should be clsoed as not-a-bug and these two
> Fixes lines removed.
I think you can tweak the original PoC [1] to trigger this bug.
Setting width/height to 0x80000000 (versus 0x8000) should do the
trick. You should be able to overflow datasize while bypassing the
sanity check (width > 512 || height > 512) as width/height are signed
prior to this patch. I haven't tested it, though.
[1] https://github.com/star-sg/CVE/blob/master/CVE-2021-4206/poc.c
[2] https://starlabs.sg/advisories/21/21-4206/
> > Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> > Reported-by: Jacek Halon <jacek.halon@gmail.com>
> > ---
> > include/ui/console.h | 4 ++--
> > ui/cursor.c | 2 +-
> > 2 files changed, 3 insertions(+), 3 deletions(-)
>
> Even though it isn't fixing a bug, the change itself still makes
> sense, because there's no reason a negative width/height is ever
> appropriate. This protects us against accidentally introducing
> future bugs, so with the two CVE Fixes lines removed:
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
>
> >
> > diff --git a/include/ui/console.h b/include/ui/console.h
> > index 2a8fab091f..92a4d90a1b 100644
> > --- a/include/ui/console.h
> > +++ b/include/ui/console.h
> > @@ -144,13 +144,13 @@ typedef struct QemuUIInfo {
> >
> > /* cursor data format is 32bit RGBA */
> > typedef struct QEMUCursor {
> > - int width, height;
> > + uint32_t width, height;
> > int hot_x, hot_y;
> > int refcount;
> > uint32_t data[];
> > } QEMUCursor;
> >
> > -QEMUCursor *cursor_alloc(int width, int height);
> > +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height);
> > QEMUCursor *cursor_ref(QEMUCursor *c);
> > void cursor_unref(QEMUCursor *c);
> > QEMUCursor *cursor_builtin_hidden(void);
> > diff --git a/ui/cursor.c b/ui/cursor.c
> > index 6fe67990e2..b5fcb64839 100644
> > --- a/ui/cursor.c
> > +++ b/ui/cursor.c
> > @@ -90,7 +90,7 @@ QEMUCursor *cursor_builtin_left_ptr(void)
> > return cursor_parse_xpm(cursor_left_ptr_xpm);
> > }
> >
> > -QEMUCursor *cursor_alloc(int width, int height)
> > +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height)
> > {
> > QEMUCursor *c;
> > size_t datasize = width * height * sizeof(uint32_t);
> > --
> > 2.40.1
> >
> >
>
> With regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
On Tue, May 23, 2023 at 02:50:09PM +0200, Mauro Matteo Cascella wrote:
> On Tue, May 23, 2023 at 10:16 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Mon, May 08, 2023 at 04:18:13PM +0200, Mauro Matteo Cascella wrote:
> > > The cursor_alloc function still accepts a signed integer for both the cursor
> > > width and height. A specially crafted negative width/height could make datasize
> > > wrap around and cause the next allocation to be 0, potentially leading to a
> > > heap buffer overflow. Modify QEMUCursor struct and cursor_alloc prototype to
> > > accept unsigned ints.
> > >
> > I concur with Marc-Andre that there is no code path that can
> > actually trigger an overflow:
> >
> >
> > hw/display/ati.c: s->cursor = cursor_alloc(64, 64);
> > hw/display/vhost-user-gpu.c: s->current_cursor = cursor_alloc(64, 64);
> > hw/display/virtio-gpu.c: s->current_cursor = cursor_alloc(64, 64);
> >
> > Not exploitable as fixed size
> >
> > hw/display/qxl-render.c: c = cursor_alloc(cursor->header.width, cursor->header.height);
> >
> > Cursor header defined as:
> >
> > typedef struct SPICE_ATTR_PACKED QXLCursorHeader {
> > uint64_t unique;
> > uint16_t type;
> > uint16_t width;
> > uint16_t height;
> > uint16_t hot_spot_x;
> > uint16_t hot_spot_y;
> > } QXLCursorHeader;
> >
> > So no negative values can be passed to cursor_alloc()
> >
> > > Fixes: CVE-2023-1601
> > > Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc (CVE-2021-4206)")
> >
> > Given there is no possible codepath that can overflow, CVE-2023-1601
> > looks invalid to me. It should be clsoed as not-a-bug and these two
> > Fixes lines removed.
>
> I think you can tweak the original PoC [1] to trigger this bug.
> Setting width/height to 0x80000000 (versus 0x8000) should do the
> trick. You should be able to overflow datasize while bypassing the
> sanity check (width > 512 || height > 512) as width/height are signed
> prior to this patch. I haven't tested it, though.
The QXLCursorHeader width/height fields are uint16_t, so 0x80000000
will get truncated. No matter what value the guest sets, when we
interpret this in qxl_cursor when calling cursor_alloc, the value
will be in the range 0-65535, as that's the bounds of uint16_t.
We'll pass this unsigned value to cursor_alloc() which converts from
uint16_t, to (signed) int. 'int' is larger than uint16_t, so the
result will still be positive in the range 0-65535, and so the sanity
check > 512 will fire and protect us.
I still see no bug, let alone a CVE.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Tue, May 23, 2023 at 3:03 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Tue, May 23, 2023 at 02:50:09PM +0200, Mauro Matteo Cascella wrote:
> > On Tue, May 23, 2023 at 10:16 AM Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > On Mon, May 08, 2023 at 04:18:13PM +0200, Mauro Matteo Cascella wrote:
> > > > The cursor_alloc function still accepts a signed integer for both the cursor
> > > > width and height. A specially crafted negative width/height could make datasize
> > > > wrap around and cause the next allocation to be 0, potentially leading to a
> > > > heap buffer overflow. Modify QEMUCursor struct and cursor_alloc prototype to
> > > > accept unsigned ints.
> > > >
> > > I concur with Marc-Andre that there is no code path that can
> > > actually trigger an overflow:
> > >
> > >
> > > hw/display/ati.c: s->cursor = cursor_alloc(64, 64);
> > > hw/display/vhost-user-gpu.c: s->current_cursor = cursor_alloc(64, 64);
> > > hw/display/virtio-gpu.c: s->current_cursor = cursor_alloc(64, 64);
> > >
> > > Not exploitable as fixed size
> > >
> > > hw/display/qxl-render.c: c = cursor_alloc(cursor->header.width, cursor->header.height);
> > >
> > > Cursor header defined as:
> > >
> > > typedef struct SPICE_ATTR_PACKED QXLCursorHeader {
> > > uint64_t unique;
> > > uint16_t type;
> > > uint16_t width;
> > > uint16_t height;
> > > uint16_t hot_spot_x;
> > > uint16_t hot_spot_y;
> > > } QXLCursorHeader;
> > >
> > > So no negative values can be passed to cursor_alloc()
>
> > >
> > > > Fixes: CVE-2023-1601
> > > > Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc (CVE-2021-4206)")
> > >
> > > Given there is no possible codepath that can overflow, CVE-2023-1601
> > > looks invalid to me. It should be clsoed as not-a-bug and these two
> > > Fixes lines removed.
> >
> > I think you can tweak the original PoC [1] to trigger this bug.
> > Setting width/height to 0x80000000 (versus 0x8000) should do the
> > trick. You should be able to overflow datasize while bypassing the
> > sanity check (width > 512 || height > 512) as width/height are signed
> > prior to this patch. I haven't tested it, though.
>
> The QXLCursorHeader width/height fields are uint16_t, so 0x80000000
> will get truncated. No matter what value the guest sets, when we
> interpret this in qxl_cursor when calling cursor_alloc, the value
> will be in the range 0-65535, as that's the bounds of uint16_t.
>
> We'll pass this unsigned value to cursor_alloc() which converts from
> uint16_t, to (signed) int. 'int' is larger than uint16_t, so the
> result will still be positive in the range 0-65535, and so the sanity
> check > 512 will fire and protect us.
Oh, you are right! Then yes, feel free to drop the two 'Fixes' lines.
This is more of a hardening bug than a real security issue. I'll
reject the newly assigned CVE.
Thanks,
> I still see no bug, let alone a CVE.
>
>
> With regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
On Mon, May 8, 2023 at 4:20 PM Mauro Matteo Cascella
<mcascell@redhat.com> wrote:
>
> The cursor_alloc function still accepts a signed integer for both the cursor
> width and height. A specially crafted negative width/height could make datasize
> wrap around and cause the next allocation to be 0, potentially leading to a
> heap buffer overflow. Modify QEMUCursor struct and cursor_alloc prototype to
> accept unsigned ints.
>
> Fixes: CVE-2023-1601
> Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc (CVE-2021-4206)")
> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> Reported-by: Jacek Halon <jacek.halon@gmail.com>
Addendum:
Reported-by: Yair Mizrahi <yairh33@gmail.com>
Reported-by: Elsayed El-Refa'ei <e.elrefaei99@gmail.com>
> ---
> include/ui/console.h | 4 ++--
> ui/cursor.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 2a8fab091f..92a4d90a1b 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -144,13 +144,13 @@ typedef struct QemuUIInfo {
>
> /* cursor data format is 32bit RGBA */
> typedef struct QEMUCursor {
> - int width, height;
> + uint32_t width, height;
> int hot_x, hot_y;
> int refcount;
> uint32_t data[];
> } QEMUCursor;
>
> -QEMUCursor *cursor_alloc(int width, int height);
> +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height);
> QEMUCursor *cursor_ref(QEMUCursor *c);
> void cursor_unref(QEMUCursor *c);
> QEMUCursor *cursor_builtin_hidden(void);
> diff --git a/ui/cursor.c b/ui/cursor.c
> index 6fe67990e2..b5fcb64839 100644
> --- a/ui/cursor.c
> +++ b/ui/cursor.c
> @@ -90,7 +90,7 @@ QEMUCursor *cursor_builtin_left_ptr(void)
> return cursor_parse_xpm(cursor_left_ptr_xpm);
> }
>
> -QEMUCursor *cursor_alloc(int width, int height)
> +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height)
> {
> QEMUCursor *c;
> size_t datasize = width * height * sizeof(uint32_t);
> --
> 2.40.1
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
08.05.2023 17:18, Mauro Matteo Cascella wrote:
> The cursor_alloc function still accepts a signed integer for both the cursor
> width and height. A specially crafted negative width/height could make datasize
> wrap around and cause the next allocation to be 0, potentially leading to a
> heap buffer overflow. Modify QEMUCursor struct and cursor_alloc prototype to
> accept unsigned ints.
>
> Fixes: CVE-2023-1601
> Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc (CVE-2021-4206)")
Looks like -stable material too?
Thanks,
/mjt
Hi
On Mon, May 8, 2023 at 6:21 PM Mauro Matteo Cascella <mcascell@redhat.com>
wrote:
> The cursor_alloc function still accepts a signed integer for both the
> cursor
> width and height. A specially crafted negative width/height could make
> datasize
> wrap around and cause the next allocation to be 0, potentially leading to a
> heap buffer overflow. Modify QEMUCursor struct and cursor_alloc prototype
> to
> accept unsigned ints.
>
> Fixes: CVE-2023-1601
> Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc
> (CVE-2021-4206)")
> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com>
> Reported-by: Jacek Halon <jacek.halon@gmail.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
It looks like this is not exploitable, QXL code uses u16 types, and VMWare
VGA checks for values > 256. Other paths use fixed size.
---
> include/ui/console.h | 4 ++--
> ui/cursor.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 2a8fab091f..92a4d90a1b 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -144,13 +144,13 @@ typedef struct QemuUIInfo {
>
> /* cursor data format is 32bit RGBA */
> typedef struct QEMUCursor {
> - int width, height;
> + uint32_t width, height;
> int hot_x, hot_y;
> int refcount;
> uint32_t data[];
> } QEMUCursor;
>
> -QEMUCursor *cursor_alloc(int width, int height);
> +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height);
> QEMUCursor *cursor_ref(QEMUCursor *c);
> void cursor_unref(QEMUCursor *c);
> QEMUCursor *cursor_builtin_hidden(void);
> diff --git a/ui/cursor.c b/ui/cursor.c
> index 6fe67990e2..b5fcb64839 100644
> --- a/ui/cursor.c
> +++ b/ui/cursor.c
> @@ -90,7 +90,7 @@ QEMUCursor *cursor_builtin_left_ptr(void)
> return cursor_parse_xpm(cursor_left_ptr_xpm);
> }
>
> -QEMUCursor *cursor_alloc(int width, int height)
> +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height)
> {
> QEMUCursor *c;
> size_t datasize = width * height * sizeof(uint32_t);
> --
> 2.40.1
>
>
>
--
Marc-André Lureau
On 9/5/23 09:13, Marc-André Lureau wrote:
> Hi
>
> On Mon, May 8, 2023 at 6:21 PM Mauro Matteo Cascella
> <mcascell@redhat.com <mailto:mcascell@redhat.com>> wrote:
>
> The cursor_alloc function still accepts a signed integer for both
> the cursor
> width and height. A specially crafted negative width/height could
> make datasize
> wrap around and cause the next allocation to be 0, potentially
> leading to a
> heap buffer overflow. Modify QEMUCursor struct and cursor_alloc
> prototype to
> accept unsigned ints.
>
> Fixes: CVE-2023-1601
> Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc
> (CVE-2021-4206)")
> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com
> <mailto:mcascell@redhat.com>>
> Reported-by: Jacek Halon <jacek.halon@gmail.com
> <mailto:jacek.halon@gmail.com>>
>
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com
> <mailto:marcandre.lureau@redhat.com>>
>
> It looks like this is not exploitable, QXL code uses u16 types, and
0xffff * 0xffff * 4 still overflows on 32-bit host, right?
> VMWare VGA checks for values > 256. Other paths use fixed size.
>
> ---
> include/ui/console.h | 4 ++--
> ui/cursor.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/ui/console.h b/include/ui/console.h
> index 2a8fab091f..92a4d90a1b 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -144,13 +144,13 @@ typedef struct QemuUIInfo {
>
> /* cursor data format is 32bit RGBA */
> typedef struct QEMUCursor {
> - int width, height;
> + uint32_t width, height;
> int hot_x, hot_y;
> int refcount;
> uint32_t data[];
> } QEMUCursor;
>
> -QEMUCursor *cursor_alloc(int width, int height);
> +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height);
> QEMUCursor *cursor_ref(QEMUCursor *c);
> void cursor_unref(QEMUCursor *c);
> QEMUCursor *cursor_builtin_hidden(void);
> diff --git a/ui/cursor.c b/ui/cursor.c
> index 6fe67990e2..b5fcb64839 100644
> --- a/ui/cursor.c
> +++ b/ui/cursor.c
> @@ -90,7 +90,7 @@ QEMUCursor *cursor_builtin_left_ptr(void)
> return cursor_parse_xpm(cursor_left_ptr_xpm);
> }
>
> -QEMUCursor *cursor_alloc(int width, int height)
> +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height)
> {
> QEMUCursor *c;
Can't we check width/height > 0 && <= SOME_LIMIT_THAT_MAKES_SENSE?
Maybe a 16K * 16K cursor is future proof and safe enough.
> size_t datasize = width * height * sizeof(uint32_t);
> --
> 2.40.1
>
>
>
>
> --
> Marc-André Lureau
On Mon, May 22, 2023 at 08:55:02PM +0200, Philippe Mathieu-Daudé wrote:
> On 9/5/23 09:13, Marc-André Lureau wrote:
> > Hi
> >
> > On Mon, May 8, 2023 at 6:21 PM Mauro Matteo Cascella
> > <mcascell@redhat.com <mailto:mcascell@redhat.com>> wrote:
> >
> > The cursor_alloc function still accepts a signed integer for both
> > the cursor
> > width and height. A specially crafted negative width/height could
> > make datasize
> > wrap around and cause the next allocation to be 0, potentially
> > leading to a
> > heap buffer overflow. Modify QEMUCursor struct and cursor_alloc
> > prototype to
> > accept unsigned ints.
> >
> > Fixes: CVE-2023-1601
> > Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc
> > (CVE-2021-4206)")
> > Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com
> > <mailto:mcascell@redhat.com>>
> > Reported-by: Jacek Halon <jacek.halon@gmail.com
> > <mailto:jacek.halon@gmail.com>>
> >
> >
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com
> > <mailto:marcandre.lureau@redhat.com>>
> >
> > It looks like this is not exploitable, QXL code uses u16 types, and
>
> 0xffff * 0xffff * 4 still overflows on 32-bit host, right?
cursor_alloc() will reject 0xffff:
if (width > 512 || height > 512) {
return NULL;
}
>
> > VMWare VGA checks for values > 256. Other paths use fixed size.
> >
> > ---
> > include/ui/console.h | 4 ++--
> > ui/cursor.c | 2 +-
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/ui/console.h b/include/ui/console.h
> > index 2a8fab091f..92a4d90a1b 100644
> > --- a/include/ui/console.h
> > +++ b/include/ui/console.h
> > @@ -144,13 +144,13 @@ typedef struct QemuUIInfo {
> >
> > /* cursor data format is 32bit RGBA */
> > typedef struct QEMUCursor {
> > - int width, height;
> > + uint32_t width, height;
> > int hot_x, hot_y;
> > int refcount;
> > uint32_t data[];
> > } QEMUCursor;
> >
> > -QEMUCursor *cursor_alloc(int width, int height);
> > +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height);
> > QEMUCursor *cursor_ref(QEMUCursor *c);
> > void cursor_unref(QEMUCursor *c);
> > QEMUCursor *cursor_builtin_hidden(void);
> > diff --git a/ui/cursor.c b/ui/cursor.c
> > index 6fe67990e2..b5fcb64839 100644
> > --- a/ui/cursor.c
> > +++ b/ui/cursor.c
> > @@ -90,7 +90,7 @@ QEMUCursor *cursor_builtin_left_ptr(void)
> > return cursor_parse_xpm(cursor_left_ptr_xpm);
> > }
> >
> > -QEMUCursor *cursor_alloc(int width, int height)
> > +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height)
> > {
> > QEMUCursor *c;
>
> Can't we check width/height > 0 && <= SOME_LIMIT_THAT_MAKES_SENSE?
>
> Maybe a 16K * 16K cursor is future proof and safe enough.
>
> > size_t datasize = width * height * sizeof(uint32_t);
> > -- 2.40.1
> >
> >
> >
> >
> > --
> > Marc-André Lureau
>
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 23/5/23 10:09, Daniel P. Berrangé wrote:
> On Mon, May 22, 2023 at 08:55:02PM +0200, Philippe Mathieu-Daudé wrote:
>> On 9/5/23 09:13, Marc-André Lureau wrote:
>>> Hi
>>>
>>> On Mon, May 8, 2023 at 6:21 PM Mauro Matteo Cascella
>>> <mcascell@redhat.com <mailto:mcascell@redhat.com>> wrote:
>>>
>>> The cursor_alloc function still accepts a signed integer for both
>>> the cursor
>>> width and height. A specially crafted negative width/height could
>>> make datasize
>>> wrap around and cause the next allocation to be 0, potentially
>>> leading to a
>>> heap buffer overflow. Modify QEMUCursor struct and cursor_alloc
>>> prototype to
>>> accept unsigned ints.
>>>
>>> Fixes: CVE-2023-1601
>>> Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc
>>> (CVE-2021-4206)")
>>> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com
>>> <mailto:mcascell@redhat.com>>
>>> Reported-by: Jacek Halon <jacek.halon@gmail.com
>>> <mailto:jacek.halon@gmail.com>>
>>>
>>>
>>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com
>>> <mailto:marcandre.lureau@redhat.com>>
>>>
>>> It looks like this is not exploitable, QXL code uses u16 types, and
>>
>> 0xffff * 0xffff * 4 still overflows on 32-bit host, right?
>
> cursor_alloc() will reject 0xffff:
>
> if (width > 512 || height > 512) {
> return NULL;
> }
I hadn't looked at the source file (the 'datasize' assignation
made me incorrectly think it'd be use before sanitized).
Still I wonder why can't we use a simple 'unsigned' type instead
of a uint32_t, but I won't insist.
>>
>>> VMWare VGA checks for values > 256. Other paths use fixed size.
>>>
>>> ---
>>> include/ui/console.h | 4 ++--
>>> ui/cursor.c | 2 +-
>>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/ui/console.h b/include/ui/console.h
>>> index 2a8fab091f..92a4d90a1b 100644
>>> --- a/include/ui/console.h
>>> +++ b/include/ui/console.h
>>> @@ -144,13 +144,13 @@ typedef struct QemuUIInfo {
>>>
>>> /* cursor data format is 32bit RGBA */
>>> typedef struct QEMUCursor {
>>> - int width, height;
>>> + uint32_t width, height;
>>> int hot_x, hot_y;
>>> int refcount;
>>> uint32_t data[];
>>> } QEMUCursor;
>>>
>>> -QEMUCursor *cursor_alloc(int width, int height);
>>> +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height);
>>> QEMUCursor *cursor_ref(QEMUCursor *c);
>>> void cursor_unref(QEMUCursor *c);
>>> QEMUCursor *cursor_builtin_hidden(void);
>>> diff --git a/ui/cursor.c b/ui/cursor.c
>>> index 6fe67990e2..b5fcb64839 100644
>>> --- a/ui/cursor.c
>>> +++ b/ui/cursor.c
>>> @@ -90,7 +90,7 @@ QEMUCursor *cursor_builtin_left_ptr(void)
>>> return cursor_parse_xpm(cursor_left_ptr_xpm);
>>> }
>>>
>>> -QEMUCursor *cursor_alloc(int width, int height)
>>> +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height)
>>> {
>>> QEMUCursor *c;
>>
>> Can't we check width/height > 0 && <= SOME_LIMIT_THAT_MAKES_SENSE?
>>
>> Maybe a 16K * 16K cursor is future proof and safe enough.
>>
>>> size_t datasize = width * height * sizeof(uint32_t);
-------------------------------^
>>> -- 2.40.1
>>>
>>>
>>>
>>>
>>> --
>>> Marc-André Lureau
>>
>>
>
> With regards,
> Daniel
On Tue, May 23, 2023 at 10:37 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> On 23/5/23 10:09, Daniel P. Berrangé wrote:
> > On Mon, May 22, 2023 at 08:55:02PM +0200, Philippe Mathieu-Daudé wrote:
> >> On 9/5/23 09:13, Marc-André Lureau wrote:
> >>> Hi
> >>>
> >>> On Mon, May 8, 2023 at 6:21 PM Mauro Matteo Cascella
> >>> <mcascell@redhat.com <mailto:mcascell@redhat.com>> wrote:
> >>>
> >>> The cursor_alloc function still accepts a signed integer for both
> >>> the cursor
> >>> width and height. A specially crafted negative width/height could
> >>> make datasize
> >>> wrap around and cause the next allocation to be 0, potentially
> >>> leading to a
> >>> heap buffer overflow. Modify QEMUCursor struct and cursor_alloc
> >>> prototype to
> >>> accept unsigned ints.
> >>>
> >>> Fixes: CVE-2023-1601
> >>> Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc
> >>> (CVE-2021-4206)")
> >>> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com
> >>> <mailto:mcascell@redhat.com>>
> >>> Reported-by: Jacek Halon <jacek.halon@gmail.com
> >>> <mailto:jacek.halon@gmail.com>>
> >>>
> >>>
> >>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com
> >>> <mailto:marcandre.lureau@redhat.com>>
> >>>
> >>> It looks like this is not exploitable, QXL code uses u16 types, and
> >>
> >> 0xffff * 0xffff * 4 still overflows on 32-bit host, right?
> >
> > cursor_alloc() will reject 0xffff:
> >
> > if (width > 512 || height > 512) {
> > return NULL;
> > }
>
> I hadn't looked at the source file (the 'datasize' assignation
> made me incorrectly think it'd be use before sanitized).
>
> Still I wonder why can't we use a simple 'unsigned' type instead
> of a uint32_t, but I won't insist.
I can send v2 with s/uint32_t/uint16_t/ if you think it's a relevant change.
> >>
> >>> VMWare VGA checks for values > 256. Other paths use fixed size.
> >>>
> >>> ---
> >>> include/ui/console.h | 4 ++--
> >>> ui/cursor.c | 2 +-
> >>> 2 files changed, 3 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/include/ui/console.h b/include/ui/console.h
> >>> index 2a8fab091f..92a4d90a1b 100644
> >>> --- a/include/ui/console.h
> >>> +++ b/include/ui/console.h
> >>> @@ -144,13 +144,13 @@ typedef struct QemuUIInfo {
> >>>
> >>> /* cursor data format is 32bit RGBA */
> >>> typedef struct QEMUCursor {
> >>> - int width, height;
> >>> + uint32_t width, height;
> >>> int hot_x, hot_y;
> >>> int refcount;
> >>> uint32_t data[];
> >>> } QEMUCursor;
> >>>
> >>> -QEMUCursor *cursor_alloc(int width, int height);
> >>> +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height);
> >>> QEMUCursor *cursor_ref(QEMUCursor *c);
> >>> void cursor_unref(QEMUCursor *c);
> >>> QEMUCursor *cursor_builtin_hidden(void);
> >>> diff --git a/ui/cursor.c b/ui/cursor.c
> >>> index 6fe67990e2..b5fcb64839 100644
> >>> --- a/ui/cursor.c
> >>> +++ b/ui/cursor.c
> >>> @@ -90,7 +90,7 @@ QEMUCursor *cursor_builtin_left_ptr(void)
> >>> return cursor_parse_xpm(cursor_left_ptr_xpm);
> >>> }
> >>>
> >>> -QEMUCursor *cursor_alloc(int width, int height)
> >>> +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height)
> >>> {
> >>> QEMUCursor *c;
> >>
> >> Can't we check width/height > 0 && <= SOME_LIMIT_THAT_MAKES_SENSE?
> >>
> >> Maybe a 16K * 16K cursor is future proof and safe enough.
> >>
> >>> size_t datasize = width * height * sizeof(uint32_t);
>
> -------------------------------^
>
> >>> -- 2.40.1
> >>>
> >>>
> >>>
> >>>
> >>> --
> >>> Marc-André Lureau
> >>
> >>
> >
> > With regards,
> > Daniel
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
On 23/5/23 14:57, Mauro Matteo Cascella wrote:
> On Tue, May 23, 2023 at 10:37 AM Philippe Mathieu-Daudé
> <philmd@linaro.org> wrote:
>>
>> On 23/5/23 10:09, Daniel P. Berrangé wrote:
>>> On Mon, May 22, 2023 at 08:55:02PM +0200, Philippe Mathieu-Daudé wrote:
>>>> On 9/5/23 09:13, Marc-André Lureau wrote:
>>>>> Hi
>>>>>
>>>>> On Mon, May 8, 2023 at 6:21 PM Mauro Matteo Cascella
>>>>> <mcascell@redhat.com <mailto:mcascell@redhat.com>> wrote:
>>>>>
>>>>> The cursor_alloc function still accepts a signed integer for both
>>>>> the cursor
>>>>> width and height. A specially crafted negative width/height could
>>>>> make datasize
>>>>> wrap around and cause the next allocation to be 0, potentially
>>>>> leading to a
>>>>> heap buffer overflow. Modify QEMUCursor struct and cursor_alloc
>>>>> prototype to
>>>>> accept unsigned ints.
>>>>>
>>>>> Fixes: CVE-2023-1601
>>>>> Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc
>>>>> (CVE-2021-4206)")
>>>>> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com
>>>>> <mailto:mcascell@redhat.com>>
>>>>> Reported-by: Jacek Halon <jacek.halon@gmail.com
>>>>> <mailto:jacek.halon@gmail.com>>
>>>>>
>>>>>
>>>>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com
>>>>> <mailto:marcandre.lureau@redhat.com>>
>>>>>
>>>>> It looks like this is not exploitable, QXL code uses u16 types, and
>>>>
>>>> 0xffff * 0xffff * 4 still overflows on 32-bit host, right?
>>>
>>> cursor_alloc() will reject 0xffff:
>>>
>>> if (width > 512 || height > 512) {
>>> return NULL;
>>> }
>>
>> I hadn't looked at the source file (the 'datasize' assignation
>> made me incorrectly think it'd be use before sanitized).
>>
>> Still I wonder why can't we use a simple 'unsigned' type instead
>> of a uint32_t, but I won't insist.
>
> I can send v2 with s/uint32_t/uint16_t/ if you think it's a relevant change.
Specifying the word size doesn't really add any (security) value IMHO.
I'll stop bikeshedding here.
Regards,
Phil.
On Tue, May 23, 2023 at 4:07 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> On 23/5/23 14:57, Mauro Matteo Cascella wrote:
> > On Tue, May 23, 2023 at 10:37 AM Philippe Mathieu-Daudé
> > <philmd@linaro.org> wrote:
> >>
> >> On 23/5/23 10:09, Daniel P. Berrangé wrote:
> >>> On Mon, May 22, 2023 at 08:55:02PM +0200, Philippe Mathieu-Daudé wrote:
> >>>> On 9/5/23 09:13, Marc-André Lureau wrote:
> >>>>> Hi
> >>>>>
> >>>>> On Mon, May 8, 2023 at 6:21 PM Mauro Matteo Cascella
> >>>>> <mcascell@redhat.com <mailto:mcascell@redhat.com>> wrote:
> >>>>>
> >>>>> The cursor_alloc function still accepts a signed integer for both
> >>>>> the cursor
> >>>>> width and height. A specially crafted negative width/height could
> >>>>> make datasize
> >>>>> wrap around and cause the next allocation to be 0, potentially
> >>>>> leading to a
> >>>>> heap buffer overflow. Modify QEMUCursor struct and cursor_alloc
> >>>>> prototype to
> >>>>> accept unsigned ints.
> >>>>>
> >>>>> Fixes: CVE-2023-1601
> >>>>> Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc
> >>>>> (CVE-2021-4206)")
> >>>>> Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com
> >>>>> <mailto:mcascell@redhat.com>>
> >>>>> Reported-by: Jacek Halon <jacek.halon@gmail.com
> >>>>> <mailto:jacek.halon@gmail.com>>
> >>>>>
> >>>>>
> >>>>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com
> >>>>> <mailto:marcandre.lureau@redhat.com>>
> >>>>>
> >>>>> It looks like this is not exploitable, QXL code uses u16 types, and
> >>>>
> >>>> 0xffff * 0xffff * 4 still overflows on 32-bit host, right?
> >>>
> >>> cursor_alloc() will reject 0xffff:
> >>>
> >>> if (width > 512 || height > 512) {
> >>> return NULL;
> >>> }
> >>
> >> I hadn't looked at the source file (the 'datasize' assignation
> >> made me incorrectly think it'd be use before sanitized).
> >>
> >> Still I wonder why can't we use a simple 'unsigned' type instead
> >> of a uint32_t, but I won't insist.
> >
> > I can send v2 with s/uint32_t/uint16_t/ if you think it's a relevant change.
>
> Specifying the word size doesn't really add any (security) value IMHO.
No security benefit, I know, it just seems more reasonable given what
Gerd said about 512x512 sprites.
> I'll stop bikeshedding here.
>
> Regards,
>
> Phil.
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
> > -QEMUCursor *cursor_alloc(int width, int height)
> > +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height)
> > {
> > QEMUCursor *c;
>
> Can't we check width/height > 0 && <= SOME_LIMIT_THAT_MAKES_SENSE?
>
> Maybe a 16K * 16K cursor is future proof and safe enough.
Modern physical hardware typically uses 512x512 sprites (even if only a
fraction of that is actually needed and >90% are just transparent pixels).
take care,
Gerd
On Mon, May 22, 2023 at 8:55 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> On 9/5/23 09:13, Marc-André Lureau wrote:
> > Hi
> >
> > On Mon, May 8, 2023 at 6:21 PM Mauro Matteo Cascella
> > <mcascell@redhat.com <mailto:mcascell@redhat.com>> wrote:
> >
> > The cursor_alloc function still accepts a signed integer for both
> > the cursor
> > width and height. A specially crafted negative width/height could
> > make datasize
> > wrap around and cause the next allocation to be 0, potentially
> > leading to a
> > heap buffer overflow. Modify QEMUCursor struct and cursor_alloc
> > prototype to
> > accept unsigned ints.
> >
> > Fixes: CVE-2023-1601
> > Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc
> > (CVE-2021-4206)")
> > Signed-off-by: Mauro Matteo Cascella <mcascell@redhat.com
> > <mailto:mcascell@redhat.com>>
> > Reported-by: Jacek Halon <jacek.halon@gmail.com
> > <mailto:jacek.halon@gmail.com>>
> >
> >
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com
> > <mailto:marcandre.lureau@redhat.com>>
> >
> > It looks like this is not exploitable, QXL code uses u16 types, and
>
> 0xffff * 0xffff * 4 still overflows on 32-bit host, right?
>
> > VMWare VGA checks for values > 256. Other paths use fixed size.
> >
> > ---
> > include/ui/console.h | 4 ++--
> > ui/cursor.c | 2 +-
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/ui/console.h b/include/ui/console.h
> > index 2a8fab091f..92a4d90a1b 100644
> > --- a/include/ui/console.h
> > +++ b/include/ui/console.h
> > @@ -144,13 +144,13 @@ typedef struct QemuUIInfo {
> >
> > /* cursor data format is 32bit RGBA */
> > typedef struct QEMUCursor {
> > - int width, height;
> > + uint32_t width, height;
> > int hot_x, hot_y;
> > int refcount;
> > uint32_t data[];
> > } QEMUCursor;
> >
> > -QEMUCursor *cursor_alloc(int width, int height);
> > +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height);
> > QEMUCursor *cursor_ref(QEMUCursor *c);
> > void cursor_unref(QEMUCursor *c);
> > QEMUCursor *cursor_builtin_hidden(void);
> > diff --git a/ui/cursor.c b/ui/cursor.c
> > index 6fe67990e2..b5fcb64839 100644
> > --- a/ui/cursor.c
> > +++ b/ui/cursor.c
> > @@ -90,7 +90,7 @@ QEMUCursor *cursor_builtin_left_ptr(void)
> > return cursor_parse_xpm(cursor_left_ptr_xpm);
> > }
> >
> > -QEMUCursor *cursor_alloc(int width, int height)
> > +QEMUCursor *cursor_alloc(uint32_t width, uint32_t height)
> > {
> > QEMUCursor *c;
>
> Can't we check width/height > 0 && <= SOME_LIMIT_THAT_MAKES_SENSE?
We currently ensure width/height are less than 512 in cursor_alloc.
Checking for positive values is unnecessary if we make width/height
unsigned, isn't it?
> Maybe a 16K * 16K cursor is future proof and safe enough.
>
> > size_t datasize = width * height * sizeof(uint32_t);
> > --
> > 2.40.1
> >
> >
> >
> >
> > --
> > Marc-André Lureau
>
--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0
© 2016 - 2024 Red Hat, Inc.