[PATCH] Fix handling of SendEvent requests which set the SendEvent bit before sending the event down the wire

[Peter Hutterer]

On Tue, Sep 13, 2011 at 07:31:12PM +0800, Sam Spilsbury wrote:
> Hi,
> 
> I haven't contributed any patches to X11 before so excuse me if I'm
> terribly in the wrong.
> 
> I noticed that the server is not removing the SendEvent (0x80) bit in
> ProcSendEvent in dix/events.c before doing range checks on the event
> type. This would cause these range checks to return an invalid BadValue
> error.
> 
> I am not sure if this is *really* a bug in the server or a bug in
> extension libraries (like libXext) who set the SendEvent bit before
> converting the event to wire format, since ProcSendEvent sets the
> SendEvent bit anyways just before it writes the event to the client.
> 
> This at least fixed a case for me where sending synthetic ShapeNotify
> events to clients resulted in a BadValue error.
> 
> Anyways, patch attached,
> 
> Thanks,
> 
> Sam

> From 4ce5d1cab01b911ab9672dbd1e773faace62e243 Mon Sep 17 00:00:00 2001
> From: Sam Spilsbury <sam.spilsbury at canonical.com>
> Date: Tue, 13 Sep 2011 19:17:56 +0800
> Subject: [PATCH] Fix SendEvent requests coming from extensions which set 0x80
>  being invalid.
> 
> Some (broken?) extension libraries set the SendEvent "magic" bit in the
> event->type field before sending the request down the wire, so when we
> did a range check on event->type it is possible that it could have been
> invalid (this is at least the case for XShape). As such, we should remove 0x80
> from the bitfield before doing a range check on the event. This is safe
> since we will re-set 0x80 on the bitfield after checking the event
> before writing it to the client.
> ---
>  dix/events.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/dix/events.c b/dix/events.c
> index 8a4c6b9..cf24869 100644
> --- a/dix/events.c
> +++ b/dix/events.c
> @@ -5241,6 +5241,17 @@ ProcSendEvent(ClientPtr client)
>  
>      REQUEST_SIZE_MATCH(xSendEventReq);
>  
> +    /* libXext and other extension libraries may set the bit indicating
> +     * that this event came from a SendEvent request so remove it
> +     * since otherwise the event type may fail the range checks
> +     * and cause an invalid BadValue error to be returned.
> +     *
> +     * This is safe to do since we later add the SendEvent bit (0x80)
> +     * back in once we send the event to the client */
> +
> +    if (stuff->event.u.u.type & 0x80)
> +	stuff->event.u.u.type &= ~(0x80);
> +

I wouldn't mind if you added a define for this to make the code a bit more
obvious. other than that, see jamey's comments. happy to merge that once
addressed.

Cheers,
  Peter

>      /* The client's event type must be a core event type or one defined by an
>  	extension. */
>  
> -- 
> 1.7.5.4
> 

> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel