Bug 59939 - TouchCopyValuatorData may go past the end of ev->valuators.data
Summary: TouchCopyValuatorData may go past the end of ev->valuators.data
Alias: None
Product: xorg
Classification: Unclassified
Component: Server/Input/Core (show other bugs)
Version: git
Hardware: All All
: low normal
Assignee: Xorg Project Team
QA Contact: Xorg Project Team
Depends on:
Reported: 2013-01-27 19:08 UTC by Alan Coopersmith
Modified: 2013-02-15 01:49 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Description Alan Coopersmith 2013-01-27 19:08:36 UTC
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;
   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.
Comment 1 Peter Hutterer 2013-01-29 01:09:03 UTC
Comment 2 Peter Hutterer 2013-02-15 01:49:25 UTC
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.