Avoid out of boundary accesses on illegal responses

The responses of the connected X server have to be properly checked
to avoid out of boundary accesses that could otherwise be triggered
by a malicious server.

From Tobias Stoeckmann / X.Org security advisory Oct 4, 2016
This commit is contained in:
matthieu 2016-10-04 15:02:31 +00:00
parent 342b1570d2
commit aebb61b811
6 changed files with 172 additions and 52 deletions

View File

@ -29,6 +29,7 @@
#include <config.h>
#endif
#include <limits.h>
#include <stdio.h>
#include <X11/Xlib.h>
/* we need to be able to manipulate the Display structure on events */
@ -272,23 +273,30 @@ static XRRScreenConfiguration *_XRRGetScreenInfo (Display *dpy,
rep.rate = 0;
rep.nrateEnts = 0;
}
if (rep.length < INT_MAX >> 2) {
nbytes = (long) rep.length << 2;
nbytes = (long) rep.length << 2;
nbytesRead = (long) (rep.nSizes * SIZEOF (xScreenSizes) +
((rep.nrateEnts + 1)& ~1) * 2 /* SIZEOF(CARD16) */);
nbytesRead = (long) (rep.nSizes * SIZEOF (xScreenSizes) +
((rep.nrateEnts + 1)& ~1) * 2 /* SIZEOF (CARD16) */);
/*
* first we must compute how much space to allocate for
* randr library's use; we'll allocate the structures in a single
* allocation, on cleanlyness grounds.
*/
/*
* first we must compute how much space to allocate for
* randr library's use; we'll allocate the structures in a single
* allocation, on cleanlyness grounds.
*/
rbytes = sizeof (XRRScreenConfiguration) +
(rep.nSizes * sizeof (XRRScreenSize) +
rep.nrateEnts * sizeof (int));
rbytes = sizeof (XRRScreenConfiguration) +
(rep.nSizes * sizeof (XRRScreenSize) +
rep.nrateEnts * sizeof (int));
scp = (struct _XRRScreenConfiguration *) Xmalloc(rbytes);
} else {
nbytes = 0;
nbytesRead = 0;
rbytes = 0;
scp = NULL;
}
scp = (struct _XRRScreenConfiguration *) Xmalloc(rbytes);
if (scp == NULL) {
_XEatData (dpy, (unsigned long) nbytes);
return NULL;

View File

@ -24,6 +24,7 @@
#include <config.h>
#endif
#include <limits.h>
#include <stdio.h>
#include <X11/Xlib.h>
/* we need to be able to manipulate the Display structure on events */
@ -57,22 +58,33 @@ XRRGetCrtcInfo (Display *dpy, XRRScreenResources *resources, RRCrtc crtc)
return NULL;
}
nbytes = (long) rep.length << 2;
if (rep.length < INT_MAX >> 2)
{
nbytes = (long) rep.length << 2;
nbytesRead = (long) (rep.nOutput * 4 +
rep.nPossibleOutput * 4);
nbytesRead = (long) (rep.nOutput * 4 +
rep.nPossibleOutput * 4);
/*
* first we must compute how much space to allocate for
* randr library's use; we'll allocate the structures in a single
* allocation, on cleanlyness grounds.
*/
/*
* first we must compute how much space to allocate for
* randr library's use; we'll allocate the structures in a single
* allocation, on cleanlyness grounds.
*/
rbytes = (sizeof (XRRCrtcInfo) +
rep.nOutput * sizeof (RROutput) +
rep.nPossibleOutput * sizeof (RROutput));
rbytes = (sizeof (XRRCrtcInfo) +
rep.nOutput * sizeof (RROutput) +
rep.nPossibleOutput * sizeof (RROutput));
xci = (XRRCrtcInfo *) Xmalloc(rbytes);
}
else
{
nbytes = 0;
nbytesRead = 0;
rbytes = 0;
xci = NULL;
}
xci = (XRRCrtcInfo *) Xmalloc(rbytes);
if (xci == NULL) {
_XEatDataWords (dpy, rep.length);
UnlockDisplay (dpy);
@ -194,12 +206,21 @@ XRRGetCrtcGamma (Display *dpy, RRCrtc crtc)
if (!_XReply (dpy, (xReply *) &rep, 0, xFalse))
goto out;
nbytes = (long) rep.length << 2;
if (rep.length < INT_MAX >> 2)
{
nbytes = (long) rep.length << 2;
/* three channels of CARD16 data */
nbytesRead = (rep.size * 2 * 3);
/* three channels of CARD16 data */
nbytesRead = (rep.size * 2 * 3);
crtc_gamma = XRRAllocGamma (rep.size);
crtc_gamma = XRRAllocGamma (rep.size);
}
else
{
nbytes = 0;
nbytesRead = 0;
crtc_gamma = NULL;
}
if (!crtc_gamma)
{
@ -357,7 +378,7 @@ XRRGetCrtcTransform (Display *dpy,
xRRGetCrtcTransformReq *req;
int major_version, minor_version;
XRRCrtcTransformAttributes *attr;
char *extra = NULL, *e;
char *extra = NULL, *end = NULL, *e;
int p;
*attributes = NULL;
@ -395,9 +416,17 @@ XRRGetCrtcTransform (Display *dpy,
else
{
int extraBytes = rep.length * 4 - CrtcTransformExtra;
extra = Xmalloc (extraBytes);
if (rep.length < INT_MAX / 4 &&
rep.length * 4 >= CrtcTransformExtra) {
extra = Xmalloc (extraBytes);
end = extra + extraBytes;
} else
extra = NULL;
if (!extra) {
_XEatDataWords (dpy, rep.length - (CrtcTransformExtra >> 2));
if (rep.length > (CrtcTransformExtra >> 2))
_XEatDataWords (dpy, rep.length - (CrtcTransformExtra >> 2));
else
_XEatDataWords (dpy, rep.length);
UnlockDisplay (dpy);
SyncHandle ();
return False;
@ -429,22 +458,38 @@ XRRGetCrtcTransform (Display *dpy,
e = extra;
if (e + rep.pendingNbytesFilter > end) {
XFree (extra);
return False;
}
memcpy (attr->pendingFilter, e, rep.pendingNbytesFilter);
attr->pendingFilter[rep.pendingNbytesFilter] = '\0';
e += (rep.pendingNbytesFilter + 3) & ~3;
for (p = 0; p < rep.pendingNparamsFilter; p++) {
INT32 f;
if (e + 4 > end) {
XFree (extra);
return False;
}
memcpy (&f, e, 4);
e += 4;
attr->pendingParams[p] = (XFixed) f;
}
attr->pendingNparams = rep.pendingNparamsFilter;
if (e + rep.currentNbytesFilter > end) {
XFree (extra);
return False;
}
memcpy (attr->currentFilter, e, rep.currentNbytesFilter);
attr->currentFilter[rep.currentNbytesFilter] = '\0';
e += (rep.currentNbytesFilter + 3) & ~3;
for (p = 0; p < rep.currentNparamsFilter; p++) {
INT32 f;
if (e + 4 > end) {
XFree (extra);
return False;
}
memcpy (&f, e, 4);
e += 4;
attr->currentParams[p] = (XFixed) f;

View File

@ -24,6 +24,7 @@
#include <config.h>
#endif
#include <limits.h>
#include <stdio.h>
#include <X11/Xlib.h>
/* we need to be able to manipulate the Display structure on events */
@ -65,6 +66,15 @@ XRRGetMonitors(Display *dpy, Window window, Bool get_active, int *nmonitors)
return NULL;
}
if (rep.length > INT_MAX >> 2 ||
rep.nmonitors > INT_MAX / SIZEOF(xRRMonitorInfo) ||
rep.noutputs > INT_MAX / 4 ||
rep.nmonitors * SIZEOF(xRRMonitorInfo) > INT_MAX - rep.noutputs * 4) {
_XEatData (dpy, rep.length);
UnlockDisplay (dpy);
SyncHandle ();
return NULL;
}
nbytes = (long) rep.length << 2;
nmon = rep.nmonitors;
noutput = rep.noutputs;
@ -111,6 +121,14 @@ XRRGetMonitors(Display *dpy, Window window, Bool get_active, int *nmonitors)
mon[m].outputs = output;
buf += SIZEOF (xRRMonitorInfo);
xoutput = (CARD32 *) buf;
if (xmon->noutput > rep.noutputs) {
Xfree(buf);
Xfree(mon);
UnlockDisplay (dpy);
SyncHandle ();
return NULL;
}
rep.noutputs -= xmon->noutput;
for (o = 0; o < xmon->noutput; o++)
output[o] = xoutput[o];
output += xmon->noutput;

View File

@ -25,6 +25,7 @@
#include <config.h>
#endif
#include <limits.h>
#include <stdio.h>
#include <X11/Xlib.h>
/* we need to be able to manipulate the Display structure on events */
@ -60,6 +61,16 @@ XRRGetOutputInfo (Display *dpy, XRRScreenResources *resources, RROutput output)
return NULL;
}
if (rep.length > INT_MAX >> 2 || rep.length < (OutputInfoExtra >> 2))
{
if (rep.length > (OutputInfoExtra >> 2))
_XEatDataWords (dpy, rep.length - (OutputInfoExtra >> 2));
else
_XEatDataWords (dpy, rep.length);
UnlockDisplay (dpy);
SyncHandle ();
return NULL;
}
nbytes = ((long) (rep.length) << 2) - OutputInfoExtra;
nbytesRead = (long) (rep.nCrtcs * 4 +

View File

@ -25,6 +25,7 @@
#include <config.h>
#endif
#include <limits.h>
#include <stdio.h>
#include <X11/Xlib.h>
/* we need to be able to manipulate the Display structure on events */
@ -59,12 +60,20 @@ XRRGetProviderResources(Display *dpy, Window window)
return NULL;
}
nbytes = (long) rep.length << 2;
if (rep.length < INT_MAX >> 2) {
nbytes = (long) rep.length << 2;
nbytesRead = (long) (rep.nProviders * 4);
nbytesRead = (long) (rep.nProviders * 4);
rbytes = (sizeof(XRRProviderResources) + rep.nProviders * sizeof(RRProvider));
xrpr = (XRRProviderResources *) Xmalloc(rbytes);
rbytes = (sizeof(XRRProviderResources) + rep.nProviders *
sizeof(RRProvider));
xrpr = (XRRProviderResources *) Xmalloc(rbytes);
} else {
nbytes = 0;
nbytesRead = 0;
rbytes = 0;
xrpr = NULL;
}
if (xrpr == NULL) {
_XEatDataWords (dpy, rep.length);
@ -121,6 +130,17 @@ XRRGetProviderInfo(Display *dpy, XRRScreenResources *resources, RRProvider provi
return NULL;
}
if (rep.length > INT_MAX >> 2 || rep.length < ProviderInfoExtra >> 2)
{
if (rep.length < ProviderInfoExtra >> 2)
_XEatDataWords (dpy, rep.length);
else
_XEatDataWords (dpy, rep.length - (ProviderInfoExtra >> 2));
UnlockDisplay (dpy);
SyncHandle ();
return NULL;
}
nbytes = ((long) rep.length << 2) - ProviderInfoExtra;
nbytesRead = (long)(rep.nCrtcs * 4 +

View File

@ -24,6 +24,7 @@
#include <config.h>
#endif
#include <limits.h>
#include <stdio.h>
#include <X11/Xlib.h>
/* we need to be able to manipulate the Display structure on events */
@ -105,27 +106,36 @@ doGetScreenResources (Display *dpy, Window window, int poll)
xrri->has_rates = _XRRHasRates (xrri->minor_version, xrri->major_version);
}
nbytes = (long) rep.length << 2;
if (rep.length < INT_MAX >> 2) {
nbytes = (long) rep.length << 2;
nbytesRead = (long) (rep.nCrtcs * 4 +
rep.nOutputs * 4 +
rep.nModes * SIZEOF (xRRModeInfo) +
((rep.nbytesNames + 3) & ~3));
nbytesRead = (long) (rep.nCrtcs * 4 +
rep.nOutputs * 4 +
rep.nModes * SIZEOF (xRRModeInfo) +
((rep.nbytesNames + 3) & ~3));
/*
* first we must compute how much space to allocate for
* randr library's use; we'll allocate the structures in a single
* allocation, on cleanlyness grounds.
*/
/*
* first we must compute how much space to allocate for
* randr library's use; we'll allocate the structures in a single
* allocation, on cleanlyness grounds.
*/
rbytes = (sizeof (XRRScreenResources) +
rep.nCrtcs * sizeof (RRCrtc) +
rep.nOutputs * sizeof (RROutput) +
rep.nModes * sizeof (XRRModeInfo) +
rep.nbytesNames + rep.nModes); /* '\0' terminate names */
rbytes = (sizeof (XRRScreenResources) +
rep.nCrtcs * sizeof (RRCrtc) +
rep.nOutputs * sizeof (RROutput) +
rep.nModes * sizeof (XRRModeInfo) +
rep.nbytesNames + rep.nModes); /* '\0' terminate names */
xrsr = (XRRScreenResources *) Xmalloc(rbytes);
wire_names = (char *) Xmalloc (rep.nbytesNames);
} else {
nbytes = 0;
nbytesRead = 0;
rbytes = 0;
xrsr = NULL;
wire_names = NULL;
}
xrsr = (XRRScreenResources *) Xmalloc(rbytes);
wire_names = (char *) Xmalloc (rep.nbytesNames);
if (xrsr == NULL || wire_names == NULL) {
if (xrsr) Xfree (xrsr);
if (wire_names) Xfree (wire_names);
@ -174,6 +184,14 @@ doGetScreenResources (Display *dpy, Window window, int poll)
wire_name = wire_names;
for (i = 0; i < rep.nModes; i++) {
xrsr->modes[i].name = names;
if (xrsr->modes[i].nameLength > rep.nbytesNames) {
Xfree (xrsr);
Xfree (wire_names);
UnlockDisplay (dpy);
SyncHandle ();
return NULL;
}
rep.nbytesNames -= xrsr->modes[i].nameLength;
memcpy (names, wire_name, xrsr->modes[i].nameLength);
names[xrsr->modes[i].nameLength] = '\0';
names += xrsr->modes[i].nameLength + 1;