Although not actually exploitable at the moment, a negative width/height could make datasize wrap around and potentially lead to buffer overflow. Since there is no reason a negative width/height is ever appropriate, modify QEMUCursor struct and cursor_alloc prototype to accept uint16_t. This protects us against accidentally introducing future bugs.
Signed-off-by: Mauro Matteo Cascella <mcasc...@redhat.com> Reported-by: Jacek Halon <jacek.ha...@gmail.com> Reported-by: Yair Mizrahi <yair...@gmail.com> Reported-by: Elsayed El-Refa'ei <e.elrefae...@gmail.com> Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com> Reviewed-by: Daniel P. Berrangé <berra...@redhat.com> --- v2: - update commit summary/description - add Reported-by/Reviewed-by - use uint16_t instead of uint32_t - add comment in cursor_alloc include/ui/console.h | 4 ++-- ui/cursor.c | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/include/ui/console.h b/include/ui/console.h index 2a8fab091f..ae5ec466c1 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; + uint16_t width, height; int hot_x, hot_y; int refcount; uint32_t data[]; } QEMUCursor; -QEMUCursor *cursor_alloc(int width, int height); +QEMUCursor *cursor_alloc(uint16_t width, uint16_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..29717b3ecb 100644 --- a/ui/cursor.c +++ b/ui/cursor.c @@ -90,11 +90,12 @@ QEMUCursor *cursor_builtin_left_ptr(void) return cursor_parse_xpm(cursor_left_ptr_xpm); } -QEMUCursor *cursor_alloc(int width, int height) +QEMUCursor *cursor_alloc(uint16_t width, uint16_t height) { QEMUCursor *c; size_t datasize = width * height * sizeof(uint32_t); + /* Modern physical hardware typically uses 512x512 sprites */ if (width > 512 || height > 512) { return NULL; } -- 2.40.1