← Back to team overview

ubuntu-x-swat team mailing list archive

[Bug 770522] Re: copyRawEvent() copies data outside of allocated space causing heap corruption

 

** Description changed:

- This fixes what I believe is a bug in libxi which causes my application
- to crash.  I have reported it to FreeDesktop.org
+ [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(). 
+ to malloc().
  
  copyRawEvent(XGenericEventCookie *cookie_in,
-              XGenericEventCookie *cookie_out)
+              XGenericEventCookie *cookie_out)
  {
-     XIRawEvent *in, *out;
-     void *ptr;
-     int len;
-     int bits;
+     XIRawEvent *in, *out;
+     void *ptr;
+     int len;
+     int bits;
  
-     in = cookie_in->data;
+     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;
+     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;
+     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 = 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->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));
+     out->raw_values = next_block(&ptr, bits * sizeof(double));
+     memcpy(out->raw_values, in->raw_values, bits * sizeof(double));
  
-     return True;
+     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

-- 
You received this bug notification because you are a member of Ubuntu-X,
which is subscribed to libxi in Ubuntu.
https://bugs.launchpad.net/bugs/770522

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


References