On Mon, May 8, 2023 at 4:20 PM Mauro Matteo Cascella
<mcasc...@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 <mcasc...@redhat.com>
> Reported-by: Jacek Halon <jacek.ha...@gmail.com>
Addendum:
Reported-by: Yair Mizrahi <yair...@gmail.com>
Reported-by: Elsayed El-Refa'ei <e.elrefae...@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
