| Summary: | TouchCopyValuatorData may go past the end of ev->valuators.data | ||
|---|---|---|---|
| Product: | xorg | Reporter: | Alan Coopersmith <alan.coopersmith> |
| Component: | Server/Input/Core | Assignee: | Xorg Project Team <xorg-team> |
| Status: | RESOLVED FIXED | QA Contact: | Xorg Project Team <xorg-team> |
| Severity: | normal | ||
| Priority: | low | CC: | chase.douglas, peter.hutterer |
| Version: | git | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
commit fdc451588816c4bc798d54e56316530e9066be80 Author: Peter Hutterer <peter.hutterer@who-t.net> Date: Tue Jan 29 11:01:29 2013 +1000 Xi: limit valuator copy to valuator array size (#59939) |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
The parfait 1.1 code analyzer reports: Error: Buffer overrun Read outside array bounds (CWE 125): In array dereference of ev->valuators.data[i] with index 'i' Array size is 36 elements (of 8 bytes each), index >= 0 and index <= 39 at line 1268 of Xi/exevents.c in function 'TouchCopyValuatorData'. which corresponds to this code: 1261 static void 1262 TouchCopyValuatorData(DeviceEvent *ev, TouchPointInfoPtr ti) 1263 { 1264 int i; 1265 1266 for (i = 0; i < sizeof(ev->valuators.mask) * 8; i++) 1267 if (BitIsOn(ev->valuators.mask, i)) 1268 valuator_mask_set_double(ti->valuators, i, ev->valuators.data[i]); 1269 } In include/eventstr.h, DeviceEvent is defined with: struct { uint8_t mask[(MAX_VALUATORS + 7) / 8];/**< Valuator mask */ uint8_t mode[(MAX_VALUATORS + 7) / 8];/**< Valuator mode (Abs or Rel)*/ double data[MAX_VALUATORS]; /**< Valuator data */ } valuators; so while mask & mode round up to match the number of bytes in a bitmask, data does not, thus leading to the 36 vs. 40 difference in array sizes. Potential fixes include changing the DeviceEvent definition to round up the number of data elements to the nearest multiple of 8 as well, or changing the loop to limit the loop counter based on the size of the data member instead of the mask member, or simply doing i < MAX_VALUATORS. Of course, as long as the masks initialize the padding bytes to 0, this shouldn't actually be hit in running code, just by code analyzers.