Re: [PATCH] ui/cursor: fix integer overflow in cursor_alloc (CVE-2022-4206)

[Mauro Matteo Cascella <mcascell@redhat.com>]

On Tue, Apr 5, 2022 at 1:10 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> > > +++ b/ui/cursor.c
> > > @@ -46,6 +46,13 @@ static QEMUCursor *cursor_parse_xpm(const char *xpm[])
> > >
> > >      /* parse pixel data */
> > >      c = cursor_alloc(width, height);
> > > +
> > > +    if (!c) {
> > > +        fprintf(stderr, "%s: cursor %ux%u alloc error\n",
> > > +                __func__, width, height);
> > > +        return NULL;
> > > +    }
> > >
> >
> > I think you could simply abort() in this function. It is used with static
> > data (ui/cursor*.xpm)
>
> Yes, that should never happen.
>
> Missing: vmsvga_cursor_define() calls cursor_alloc() with guest-supplied
> values too.

I skipped that because the check (cursor.width > 256 || cursor.height
> 256) is already done in vmsvga_fifo_run before calling
vmsvga_cursor_define. You want me to add another check in
vmsvga_cursor_define and return NULL if cursor_alloc fails?

> take care,
>   Gerd
>


--
Mauro Matteo Cascella
Red Hat Product Security
PGP-Key ID: BB3410B0