[Spice-devel] [PATCH v2] red-parse-qxl: Check consistency of QXL_DRAW_COPY operations

[Frediano Ziglio]

> 
> The source area should not extend outside the source bitmap, or have
> swapped coordinates.
> 
> Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
> ---
>  server/red-parse-qxl.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 

I checked and if this function return error the resource is correctly
released.

> diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
> index 0dafbef..d1f0ea7 100644
> --- a/server/red-parse-qxl.c
> +++ b/server/red-parse-qxl.c
> @@ -682,6 +682,18 @@ static int red_get_copy_ptr(RedMemSlotInfo *slots, int
> group_id,
>          return 1;
>      }
>      red_get_rect_ptr(&red->src_area, &qxl->src_area);
> +    /* The source area should not extend outside the source bitmap or have
> +     * swapped coordinates.
> +     */
> +    if (red->src_bitmap->descriptor.type == SPICE_IMAGE_TYPE_BITMAP &&

Why this check? I think should be valid for any kind of image, even
jpeg, lz or whatever.
Maybe would be worth to have a function to check any rect so
could be reused.

> +        (red->src_area.left < 0 ||
> +         red->src_area.left > red->src_area.right ||
> +         red->src_area.right > red->src_bitmap->u.bitmap.x ||
> +         red->src_area.top < 0 || red->src_area.top > red->src_area.bottom
> ||
> +         red->src_area.bottom > red->src_bitmap->u.bitmap.y)) {
> +        red_put_image(red->src_bitmap);

Mm... this make me think you didn't test the code.. this cause
a double free in the current code

> +        return 1;
> +    }
>      red->rop_descriptor  = qxl->rop_descriptor;
>      red->scale_mode      = qxl->scale_mode;
>      red_get_qmask_ptr(slots, group_id, &red->mask, &qxl->mask, flags);

Frediano