[Xcb] [PATCH] memcpy() with wrong size in xcb/util/icccm

[Peter Harris]

Michael Stapelberg wrote:
> The problem seems to be using the wrong length for memcpy in
> xcb_get_wm_size_hints_from_reply(), see this patch:
> 
> --- icccm/O.icccm.c	2009-12-21 19:33:35.019052049 +0100
> +++ icccm/icccm.c	2009-12-21 19:33:42.951051864 +0100
> @@ -445,8 +445,7 @@
>          length >= 15))
>      return 0;
>  
> -  memcpy(hints, (xcb_size_hints_t *) xcb_get_property_value (reply),
> -         length * reply->format >> 3);
> +  memcpy(hints, (xcb_size_hints_t *) xcb_get_property_value (reply), length);
>  
>    flags = (XCB_SIZE_HINT_US_POSITION | XCB_SIZE_HINT_US_SIZE |
>             XCB_SIZE_HINT_P_POSITION | XCB_SIZE_HINT_P_SIZE |

Well, 'length' is divided by reply->format/8 just a few lines above, so
it needs to be multiplied by reply->format/8 again to get the number of
bytes.

> I think this is the correct way to do it because it matches the
> sizeof(xcb_size_hints_t) 

Are you sure?

> and because xcb_get_wm_hints_from_reply() also does
> not multiply with (reply->format >> 3).

xcb_get_wm_hints_from_reply() does not divide by reply->format/8, which
is why it doesn't need to multiply by it again. (Well, it does, but it
calls the result num_elem).

> Still, it would be great if anyone of
> you more familiar with this code could review it and apply the patch for the
> next release of xcb-util.

There is definitely a major problem with this code: It copies an
untrusted amount of data into a fixed length buffer. See the attached
patch; does this fix the problem for you?

To the list at large (since this has been present since before the
switch to git): is there a reason WM_SIZE_HINTS was allowed to have a
format other than 32? Other formats are broken in a mixed-endianness
environment, so I can't imagine the spec ever allowed them.

Peter Harris
-- 
               Open Text Connectivity Solutions Group
Peter Harris                    http://connectivity.opentext.com/
Research and Development        Phone: +1 905 762 6001
pharris at opentext.com            Toll Free: 1 877 359 4866