From 36d6ecb9bcbe8554ab01fe4a24a19462072fdd64 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 9 Mar 2013 22:55:23 -0800 Subject: [PATCH 07/16] integer overflow in XGetDeviceControl() [CVE-2013-1984 1/8] If the number of valuators reported by the server is large enough that it overflows when multiplied by the size of the appropriate struct, then memory corruption can occur when more bytes are copied from the X server reply than the size of the buffer we allocated to hold them. v2: check that reply size fits inside the data read from the server, so we don't read out of bounds either Reported-by: Ilja Van Sprundel Signed-off-by: Alan Coopersmith Reviewed-by: Peter Hutterer (cherry picked from commit b0b13c12a8079a5a0e7f43b2b8983699057b2cec) (cherry picked from commit e7ebe26edd40996a9b36b0c7a068728781eb41bb) --- src/XGetDCtl.c | 31 ++++++++++++++++++++++++------- 1 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/XGetDCtl.c b/src/XGetDCtl.c index a1e49ee..66a7100 100644 --- a/src/XGetDCtl.c +++ b/src/XGetDCtl.c @@ -57,6 +57,7 @@ SOFTWARE. #include #include #include "XIint.h" +#include XDeviceControl * XGetDeviceControl( @@ -64,8 +65,6 @@ XGetDeviceControl( XDevice *dev, int control) { - int size = 0; - int nbytes, i; XDeviceControl *Device = NULL; XDeviceControl *Sav = NULL; xDeviceState *d = NULL; @@ -88,8 +87,12 @@ XGetDeviceControl( goto out; if (rep.length > 0) { - nbytes = (long)rep.length << 2; - d = (xDeviceState *) Xmalloc((unsigned)nbytes); + unsigned long nbytes; + size_t size = 0; + if (rep.length < (INT_MAX >> 2)) { + nbytes = (unsigned long) rep.length << 2; + d = Xmalloc(nbytes); + } if (!d) { _XEatDataWords(dpy, rep.length); goto out; @@ -107,33 +110,46 @@ XGetDeviceControl( case DEVICE_RESOLUTION: { xDeviceResolutionState *r; + size_t val_size; r = (xDeviceResolutionState *) d; - size += sizeof(XDeviceResolutionState) + - (3 * sizeof(int) * r->num_valuators); + if (r->num_valuators >= (INT_MAX / (3 * sizeof(int)))) + goto out; + val_size = 3 * sizeof(int) * r->num_valuators; + if ((sizeof(xDeviceResolutionState) + val_size) > nbytes) + goto out; + size += sizeof(XDeviceResolutionState) + val_size; break; } case DEVICE_ABS_CALIB: { + if (sizeof(xDeviceAbsCalibState) > nbytes) + goto out; size += sizeof(XDeviceAbsCalibState); break; } case DEVICE_ABS_AREA: { + if (sizeof(xDeviceAbsAreaState) > nbytes) + goto out; size += sizeof(XDeviceAbsAreaState); break; } case DEVICE_CORE: { + if (sizeof(xDeviceCoreState) > nbytes) + goto out; size += sizeof(XDeviceCoreState); break; } default: + if (d->length > nbytes) + goto out; size += d->length; break; } - Device = (XDeviceControl *) Xmalloc((unsigned)size); + Device = Xmalloc(size); if (!Device) goto out; @@ -146,6 +162,7 @@ XGetDeviceControl( int *iptr, *iptr2; xDeviceResolutionState *r; XDeviceResolutionState *R; + unsigned int i; r = (xDeviceResolutionState *) d; R = (XDeviceResolutionState *) Device; -- 1.7.7.1