Re: [PATCH v2] ui/clipboard: ensure data is available or request callback is set upon update

[Marc-André Lureau]

Hi

On Wed, Jan 17, 2024 at 3:30 PM Fiona Ebner <f.ebner@proxmox.com> wrote:
>
> Am 17.01.24 um 12:11 schrieb Marc-André Lureau:
> > Hi
> >
> > On Wed, Jan 17, 2024 at 3:01 PM Fiona Ebner <f.ebner@proxmox.com> wrote:
> >>
> >> +    for (type = 0; type < QEMU_CLIPBOARD_TYPE__COUNT && !missing_data; 
> >> type++) {
> >> +        if (!info->types[type].data) {
> >> +            missing_data = true;
> >> +        }
> >> +    }
> >> +    /*
> >> +     * If data is missing, the clipboard owner's 'request' callback needs 
> >> to be
> >> +     * set. Otherwise, there is no way to get the clipboard data and
> >> +     * qemu_clipboard_request() cannot be called.
> >> +     */
> >> +    if (missing_data && info->owner && !info->owner->request) {
> >> +        return;
> >> +    }
> >
> > It needs to check whether the type is "available". If not data is
> > provided, owner should be set as well, it should assert() that.
> >
> > That should do the job:
> >
> > for (type = 0; type < QEMU_CLIPBOARD_TYPE__COUNT; type++) {
> >     /*
> >      * If data is missing, the clipboard owner's 'request' callback needs to
> >      * be set. Otherwise, there is no way to get the clipboard data and
> >      * qemu_clipboard_request() cannot be called.
> >      */
> >     if (info->types[type].available && !info->types[type].data) {
> >         assert(info->owner && info->owner->request);
> >     }
> > }
> >
>
> Okay, thanks! But we can't assert, because that doesn't resolve the CVE
> as it would still crash. The VNC client might not have the
> VNC_FEATURE_CLIPBOARD_EXT feature, and the request callback is currently
> only set in that case. But we can return instead of assert to just avoid
> clipboard update. I'll send a v3.

If it doesn't have VNC_FEATURE_CLIPBOARD_EXT, it shouldn't update the
clipboard without data. (ClientCutText/ServerCutText always have data,
even if 0-length)

-- 
Marc-André Lureau