If the size of a VD_AGENT_CLIPBOARD_GRAB message is invalid we leak info
when returning early.

Thanks to Coverity for spotting this:

*** CID 1453266:  Resource leaks  (RESOURCE_LEAK)
/qemu/ui/vdagent.c: 465 in vdagent_chr_recv_clipboard()
459             info = qemu_clipboard_info_new(&vd->cbpeer, s);
460             if (size > sizeof(uint32_t) * 10) {
461                 /*
462                  * spice has 6 types as of 2021. Limiting to 10 entries
463                  * so we we have some wiggle room.
464                  */
>>>     CID 1453266:  Resource leaks  (RESOURCE_LEAK)
>>>     Variable "info" going out of scope leaks the storage it points to.
465                 return;
466             }
467             while (size >= sizeof(uint32_t)) {
468                 trace_vdagent_cb_grab_type(GET_NAME(type_name, *(uint32_t *)data));
469                 switch (*(uint32_t *)data) {
470                 case VD_AGENT_CLIPBOARD_UTF8_TEXT:

Fixes: f0349f4d8947ad32d0fa4678cbf5dbb356fcbda1 ("ui/vdagent: add clipboard support")
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 ui/vdagent.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/vdagent.c b/ui/vdagent.c
index a253a8fe63..8fc54d330e 100644
--- a/ui/vdagent.c
+++ b/ui/vdagent.c
@@ -456,7 +456,6 @@ static void vdagent_chr_recv_clipboard(VDAgentChardev *vd, VDAgentMessage *msg)
     switch (msg->type) {
     case VD_AGENT_CLIPBOARD_GRAB:
         trace_vdagent_cb_grab_selection(GET_NAME(sel_name, s));
-        info = qemu_clipboard_info_new(&vd->cbpeer, s);
         if (size > sizeof(uint32_t) * 10) {
             /*
              * spice has 6 types as of 2021. Limiting to 10 entries
@@ -464,6 +463,7 @@ static void vdagent_chr_recv_clipboard(VDAgentChardev *vd, VDAgentMessage *msg)
              */
             return;
         }
+        info = qemu_clipboard_info_new(&vd->cbpeer, s);
         while (size >= sizeof(uint32_t)) {
             trace_vdagent_cb_grab_type(GET_NAME(type_name, *(uint32_t *)data));
             switch (*(uint32_t *)data) {
-- 
2.31.1

On 6/1/21 5:57 PM, Stefan Hajnoczi wrote:
> If the size of a VD_AGENT_CLIPBOARD_GRAB message is invalid we leak info
> when returning early.
> 
> Thanks to Coverity for spotting this:
> 
> *** CID 1453266:  Resource leaks  (RESOURCE_LEAK)
> /qemu/ui/vdagent.c: 465 in vdagent_chr_recv_clipboard()
> 459             info = qemu_clipboard_info_new(&vd->cbpeer, s);
> 460             if (size > sizeof(uint32_t) * 10) {
> 461                 /*
> 462                  * spice has 6 types as of 2021. Limiting to 10 entries
> 463                  * so we we have some wiggle room.
> 464                  */
>>>>     CID 1453266:  Resource leaks  (RESOURCE_LEAK)
>>>>     Variable "info" going out of scope leaks the storage it points to.
> 465                 return;
> 466             }
> 467             while (size >= sizeof(uint32_t)) {
> 468                 trace_vdagent_cb_grab_type(GET_NAME(type_name, *(uint32_t *)data));
> 469                 switch (*(uint32_t *)data) {
> 470                 case VD_AGENT_CLIPBOARD_UTF8_TEXT:
> 
> Fixes: f0349f4d8947ad32d0fa4678cbf5dbb356fcbda1 ("ui/vdagent: add clipboard support")
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  ui/vdagent.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

On 01/06/2021 17.57, Stefan Hajnoczi wrote:
> If the size of a VD_AGENT_CLIPBOARD_GRAB message is invalid we leak info
> when returning early.
> 
> Thanks to Coverity for spotting this:
> 
> *** CID 1453266:  Resource leaks  (RESOURCE_LEAK)
> /qemu/ui/vdagent.c: 465 in vdagent_chr_recv_clipboard()
> 459             info = qemu_clipboard_info_new(&vd->cbpeer, s);
> 460             if (size > sizeof(uint32_t) * 10) {
> 461                 /*
> 462                  * spice has 6 types as of 2021. Limiting to 10 entries
> 463                  * so we we have some wiggle room.
> 464                  */
>>>>      CID 1453266:  Resource leaks  (RESOURCE_LEAK)
>>>>      Variable "info" going out of scope leaks the storage it points to.
> 465                 return;
> 466             }
> 467             while (size >= sizeof(uint32_t)) {
> 468                 trace_vdagent_cb_grab_type(GET_NAME(type_name, *(uint32_t *)data));
> 469                 switch (*(uint32_t *)data) {
> 470                 case VD_AGENT_CLIPBOARD_UTF8_TEXT:
> 
> Fixes: f0349f4d8947ad32d0fa4678cbf5dbb356fcbda1 ("ui/vdagent: add clipboard support")
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   ui/vdagent.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ui/vdagent.c b/ui/vdagent.c
> index a253a8fe63..8fc54d330e 100644
> --- a/ui/vdagent.c
> +++ b/ui/vdagent.c
> @@ -456,7 +456,6 @@ static void vdagent_chr_recv_clipboard(VDAgentChardev *vd, VDAgentMessage *msg)
>       switch (msg->type) {
>       case VD_AGENT_CLIPBOARD_GRAB:
>           trace_vdagent_cb_grab_selection(GET_NAME(sel_name, s));
> -        info = qemu_clipboard_info_new(&vd->cbpeer, s);
>           if (size > sizeof(uint32_t) * 10) {
>               /*
>                * spice has 6 types as of 2021. Limiting to 10 entries
> @@ -464,6 +463,7 @@ static void vdagent_chr_recv_clipboard(VDAgentChardev *vd, VDAgentMessage *msg)
>                */
>               return;
>           }
> +        info = qemu_clipboard_info_new(&vd->cbpeer, s);
>           while (size >= sizeof(uint32_t)) {
>               trace_vdagent_cb_grab_type(GET_NAME(type_name, *(uint32_t *)data));
>               switch (*(uint32_t *)data) {
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>