copyRawEvent() copies data outside of allocated space causing heap corruption

[Roger R. Cruz]

[Impact]
Causes crashes in applications using Raw Motion support from XI2

[Development Fix]
Fix is committed to upstream git tree, which we'll get when we update libXi. Meanwhile, a cherrypick of the patch has been included in our current libxi package in oneiric; this patch can be dropped when we merge from debian.

[Stable Fix]
Same cherrypick patch for oneiric is used for natty, just different package version.

[Test Case]
See test case code shown below.

[Regression Potential]
The patch fixes a fairly obvious typo in the code. It strictly increases the size of memory allocation, so there is no risk of overwriting memory due to erroneous assumptions elsewhere in the code. This patch has been upstream for a bit and received a suitable amount of testing.

[Original Report]
This fixes what I believe is a bug in libxi which causes my application to crash. I have reported it to FreeDesktop.org

https://bugs.freedesktop.org/show_bug.cgi?id=36592

diff ../../libXi-1.3-orig/src/XExtInt.c src/XExtInt.c
1196c1196
< ptr = cookie_out->data = malloc(sizeof(XIRawEvent));
---
> ptr = cookie_out->data = malloc(len);

In trying to understand a heap corruption when I added XI2 RawMotion event
handling to our Xinput-based application, I came across the following routine
copyRawEvent() in libxi-1.3/src/XExtInt.c. My question is what is the purpose
of computing "len" if it is not used? Should it have been used as an argument
to malloc().

copyRawEvent(XGenericEventCookie *cookie_in,
             XGenericEventCookie *cookie_out)
{
    XIRawEvent *in, *out;
    void *ptr;
    int len;
    int bits;

    in = cookie_in->data;

    bits = count_bits(in->valuators.mask, in->valuators.mask_len);
    len = sizeof(XIRawEvent) + in->valuators.mask_len;
    len += bits * sizeof(double) * 2;

    ptr = cookie_out->data = malloc(sizeof(XIRawEvent));
    if (!ptr)
        return False;

    out = next_block(&ptr, sizeof(XIRawEvent));
    *out = *in;
    out->valuators.mask = next_block(&ptr, out->valuators.mask_len);
    memcpy(out->valuators.mask, in->valuators.mask, out->valuators.mask_len);

    out->valuators.values = next_block(&ptr, bits * sizeof(double));
    memcpy(out->valuators.values, in->valuators.values, bits * sizeof(double));

    out->raw_values = next_block(&ptr, bits * sizeof(double));
    memcpy(out->raw_values, in->raw_values, bits * sizeof(double));

    return True;
}

When I use valgrind, I get the following output as the culprit for the crash

==4166== Invalid write of size 1
==4166== at 0x4C29F04: memcpy (mc_replace_strmem.c:497)
==4166== by 0x8F39180: ??? (in /usr/lib/libXi.so.6.1.0)
==4166== by 0x7433D48: _XCopyEventCookie (in /usr/lib/libX11.so.6.3.0)
==4166== by 0x7425166: XPeekEvent (in /usr/lib/libX11.so.6.3.0)
==4166== by 0x49C3E3: process_key (x11_be.c:1065)
==4166== by 0x49EA5C: event_key_release (x11_be.c:2201)
==4166== by 0x49DD6E: x11_be_process_events (x11_be.c:1892)
==4166== by 0x4A38F4: x11_be_main_loop (x11_be.c:4353)
==4166== by 0x4A39E1: x11_be_thread_main (x11_be.c:4385)
==4166== by 0x87549C9: start_thread (pthread_create.c:300)
==4166== by 0x8A516FC: clone (clone.S:112)
==4166== Address 0x168afe80 is 0 bytes after a block of size 96 alloc'd
==4166== at 0x4C284A8: malloc (vg_replace_malloc.c:236)
==4166== by 0x8F390BD: ??? (in /usr/lib/libXi.so.6.1.0)
==4166== by 0x7433D48: _XCopyEventCookie (in /usr/lib/libX11.so.6.3.0)
==4166== by 0x7425166: XPeekEvent (in /usr/lib/libX11.so.6.3.0)
==4166== by 0x49C3E3: process_key (x11_be.c:1065)
==4166== by 0x49EA5C: event_key_release (x11_be.c:2201)
==4166== by 0x49DD6E: x11_be_process_events (x11_be.c:1892)
==4166== by 0x4A38F4: x11_be_main_loop (x11_be.c:4353)
==4166== by 0x4A39E1: x11_be_thread_main (x11_be.c:4385)
==4166== by 0x87549C9: start_thread (pthread_create.c:300)

Thanks in advance,

Roger R. Cruz

[Bryce Harrington (bryce)]

(http://patchwork.freedesktop.org/patch/5270/)

[Launchpad Janitor (janitor)]

This bug was fixed in the package libxi - 2:1.4.1-1ubuntu3

---------------
libxi (2:1.4.1-1ubuntu3) oneiric; urgency=low

  * Add 100_len_raw_events.patch: Cherrypick upstream patch to correct the
    use of the calculated structure length for memory allocations. Fixes
    issue where incorrect memory size was allocated, leading to valuator
    data overwriting memory, resulting in an error "Invalid write of size
    1" and heap corruption causing applications to crash when they use Raw
    Events. (LP: #770522)
 -- Bryce Harrington <email address hidden> Thu, 05 May 2011 14:11:39 -0700

[Martin Pitt (pitti)]

Accepted libxi into natty-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

[dino99 (9d9)]

eol reached https://wiki.ubuntu.com/Releases