From a0df3e1c7728205e5c7650b2e6dce684139254a6 Mon Sep 17 00:00:00 2001
From: Tobias Stoeckmann <tobias@stoeckmann.org>
Date: Sun, 25 Sep 2016 22:21:40 +0200
Subject: [PATCH] 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.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Reviewed-by: Matthieu Herrb <matthieu@herrb.eu>
---
 src/XrrConfig.c   | 32 +++++++++++++--------
 src/XrrCrtc.c     | 83 ++++++++++++++++++++++++++++++++++++++++++-------------
 src/XrrMonitor.c  | 18 ++++++++++++
 src/XrrOutput.c   | 11 ++++++++
 src/XrrProvider.c | 28 ++++++++++++++++---
 src/XrrScreen.c   | 52 ++++++++++++++++++++++------------
 6 files changed, 172 insertions(+), 52 deletions(-)

Index: libXrandr-1.3.0/src/XrrConfig.c
===================================================================
--- libXrandr-1.3.0.orig/src/XrrConfig.c
+++ libXrandr-1.3.0/src/XrrConfig.c
@@ -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,28 @@ static XRRScreenConfiguration *_XRRGetSc
 	rep.rate = 0;
 	rep.nrateEnts = 0;
     }
-    
-    nbytes = (long) rep.length << 2;
-
-    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.
-     */
+    if (rep.length < INT_MAX >> 2) {
+       nbytes = (long) rep.length << 2;
 
-    rbytes = sizeof (XRRScreenConfiguration) +
-      (rep.nSizes * sizeof (XRRScreenSize) +
-       rep.nrateEnts * sizeof (int));
-
-    scp = (struct _XRRScreenConfiguration *) Xmalloc(rbytes);
+       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.
+        */
+
+       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;
+    }
     if (scp == NULL) {
 	_XEatData (dpy, (unsigned long) nbytes);
 	return NULL;
Index: libXrandr-1.3.0/src/XrrCrtc.c
===================================================================
--- libXrandr-1.3.0.orig/src/XrrCrtc.c
+++ libXrandr-1.3.0/src/XrrCrtc.c
@@ -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,29 @@ XRRGetCrtcInfo (Display *dpy, XRRScreenR
 	return NULL;
     }
 
-    nbytes = (long) rep.length << 2;
-
-    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.
-     */
+    if (rep.length < INT_MAX >> 2)
+    {
+       nbytes = (long) rep.length << 2;
 
-    rbytes = (sizeof (XRRCrtcInfo) +
-	      rep.nOutput * sizeof (RROutput) +
-	      rep.nPossibleOutput * sizeof (RROutput));
+       nbytesRead = (long) (rep.nOutput * 4 +
+                            rep.nPossibleOutput * 4);
 
-    xci = (XRRCrtcInfo *) Xmalloc(rbytes);
+       /*
+        * 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));
+       xci = (XRRCrtcInfo *) Xmalloc(rbytes);
+    } else {
+       nbytes = 0;
+       nbytesRead = 0;
+       rbytes = 0;
+       xci = NULL;
+    }
     if (xci == NULL) {
 	_XEatData (dpy, (unsigned long) nbytes);
 	UnlockDisplay (dpy);
@@ -194,13 +202,18 @@ XRRGetCrtcGamma (Display *dpy, RRCrtc cr
     if (!_XReply (dpy, (xReply *) &rep, 0, xFalse))
 	rep.status = RRSetConfigFailed;
 
-    nbytes = (long) rep.length << 2;
-    
-    /* three channels of CARD16 data */
-    nbytesRead = (rep.size * 2 * 3);
+    if (rep.length < INT_MAX >> 2)
+    {
+       nbytes = (long) rep.length << 2;
 
-    crtc_gamma = XRRAllocGamma (rep.size);
-    
+       /* three channels of CARD16 data */
+       nbytesRead = (rep.size * 2 * 3);
+       crtc_gamma = XRRAllocGamma (rep.size);
+    } else {
+       nbytes = 0;
+       nbytesRead = 0;
+       crtc_gamma = NULL;
+    }
     if (!crtc_gamma)
     {
 	_XEatData (dpy, (unsigned long) nbytes);
@@ -358,7 +371,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;
@@ -396,7 +409,12 @@ 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) {
 		_XEatData (dpy, extraBytes);
 		UnlockDisplay (dpy);
@@ -430,22 +448,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;
Index: libXrandr-1.3.0/src/XrrOutput.c
===================================================================
--- libXrandr-1.3.0.orig/src/XrrOutput.c
+++ libXrandr-1.3.0/src/XrrOutput.c
@@ -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 */
@@ -32,6 +33,7 @@
 #include <X11/extensions/render.h>
 #include <X11/extensions/Xrender.h>
 #include "Xrandrint.h"
+#include "eat.h"
 
 #define OutputInfoExtra	(SIZEOF(xRRGetOutputInfoReply) - 32)
 				
@@ -60,6 +62,16 @@ XRRGetOutputInfo (Display *dpy, XRRScree
 	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 +
Index: libXrandr-1.3.0/src/XrrScreen.c
===================================================================
--- libXrandr-1.3.0.orig/src/XrrScreen.c
+++ libXrandr-1.3.0/src/XrrScreen.c
@@ -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,37 @@ doGetScreenResources (Display *dpy, Wind
 	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));
-
-    /* 
-     * 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 */
+       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.
+        */
+
+       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 +185,14 @@ doGetScreenResources (Display *dpy, Wind
     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;
--- /dev/null	2016-08-03 12:34:20.529634699 +0200
+++ libXrandr-1.3.0/src/eat.h	2016-11-25 13:51:31.934745000 +0100
@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#ifdef HAVE_CONFIG_H
+# include "config.h"
+#endif
+
+#ifndef HAVE__XEATDATAWORDS
+#include <X11/Xmd.h>  /* for LONG64 on 64-bit platforms */
+#include <limits.h>
+
+static inline void _XEatDataWords(Display *dpy, unsigned long n)
+{
+# ifndef LONG64
+    if (n >= (ULONG_MAX >> 2))
+        _XIOError(dpy);
+# endif
+    _XEatData (dpy, n << 2);
+}
+#endif
+
