On Tue, May 23, 2023 at 4:07 PM Philippe Mathieu-Daudé <phi...@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é > > <phi...@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 > >>>>> <mcasc...@redhat.com <mailto: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 > >>>>> <mailto:mcasc...@redhat.com>> > >>>>> Reported-by: Jacek Halon <jacek.ha...@gmail.com > >>>>> <mailto:jacek.ha...@gmail.com>> > >>>>> > >>>>> > >>>>> Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com > >>>>> <mailto:marcandre.lur...@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
