From owner-svn-ports-all@FreeBSD.ORG Wed Dec 10 21:35:17 2014 Return-Path: Delivered-To: svn-ports-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 86CF4336; Wed, 10 Dec 2014 21:35:17 +0000 (UTC) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 71EF880F; Wed, 10 Dec 2014 21:35:17 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.9/8.14.9) with ESMTP id sBALZH3f029910; Wed, 10 Dec 2014 21:35:17 GMT (envelope-from kwm@FreeBSD.org) Received: (from kwm@localhost) by svn.freebsd.org (8.14.9/8.14.9/Submit) id sBALZD19029883; Wed, 10 Dec 2014 21:35:13 GMT (envelope-from kwm@FreeBSD.org) Message-Id: <201412102135.sBALZD19029883@svn.freebsd.org> X-Authentication-Warning: svn.freebsd.org: kwm set sender to kwm@FreeBSD.org using -f From: Koop Mast Date: Wed, 10 Dec 2014 21:35:13 +0000 (UTC) To: ports-committers@freebsd.org, svn-ports-all@freebsd.org, svn-ports-head@freebsd.org Subject: svn commit: r374489 - in head/x11-servers/xorg-server: . files X-SVN-Group: ports-head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-ports-all@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the ports tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Dec 2014 21:35:17 -0000 Author: kwm Date: Wed Dec 10 21:35:13 2014 New Revision: 374489 URL: https://svnweb.freebsd.org/changeset/ports/374489 QAT: https://qat.redports.org/buildarchive/r374489/ Log: Fix multiple xserver security advisories in the 1.12.4 xserver. The patches where not ported to 1.7.7 so mark it forbidden. This version is not default anymore and will be removed in the 1.14 update that currently being tested. Obtained from: xserver upstream MFH: 2014Q4 Security: 27b9b2f0-8081-11e4-b4ca-bcaec565249c Added: head/x11-servers/xorg-server/files/patch-CVE-2014-8092-1-4 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8092-2-4 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8092-3-4 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8092-4-4 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8092-pt5 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8092-pt6 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8093-1-6 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8093-2-6 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8093-3-6 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8093-4-6 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8093-5-6 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8093-6-6 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8094 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8095 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8096 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8097 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8097-pt2 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8098-1-8 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8098-2-8 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8098-3-8 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8098-4-8 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8098-6-8 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8098-7-8 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8098-8-8 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8099 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8100-1-2 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8100-2-2 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8101 (contents, props changed) head/x11-servers/xorg-server/files/patch-CVE-2014-8102 (contents, props changed) Modified: head/x11-servers/xorg-server/Makefile Modified: head/x11-servers/xorg-server/Makefile ============================================================================== --- head/x11-servers/xorg-server/Makefile Wed Dec 10 21:31:56 2014 (r374488) +++ head/x11-servers/xorg-server/Makefile Wed Dec 10 21:35:13 2014 (r374489) @@ -35,13 +35,14 @@ OPTIONS_EXCLUDE_sparc64= HAL .if defined(WITH_NEW_XORG) XORG_VERSION= 1.12.4 -XORG_REVISION= 9 +XORG_REVISION= 10 PLIST_SUB+= OLD="@comment " NEW="" EXTRA_PATCHES+= ${FILESDIR}/extra-clang \ ${FILESDIR}/extra-configure \ ${FILESDIR}/extra-new-bad-impl \ ${FILESDIR}/extra-new-dix_dixfonts.c .else +FORBIDDEN= unfixed security issues XORG_VERSION= 1.7.7 XORG_REVISION= 14 PLIST_SUB+= OLD="" NEW="@comment " Added: head/x11-servers/xorg-server/files/patch-CVE-2014-8092-1-4 ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/x11-servers/xorg-server/files/patch-CVE-2014-8092-1-4 Wed Dec 10 21:35:13 2014 (r374489) @@ -0,0 +1,37 @@ +From eeae42d60bf3d5663ea088581f6c28a82cd17829 Mon Sep 17 00:00:00 2001 +From: Alan Coopersmith +Date: Wed, 22 Jan 2014 21:11:16 -0800 +Subject: [PATCH 01/40] dix: integer overflow in ProcPutImage() [CVE-2014-8092 + 1/4] + +ProcPutImage() calculates a length field from a width, left pad and depth +specified by the client (if the specified format is XYPixmap). + +The calculations for the total amount of memory the server needs for the +pixmap can overflow a 32-bit number, causing out-of-bounds memory writes +on 32-bit systems (since the length is stored in a long int variable). + +Reported-by: Ilja Van Sprundel +Signed-off-by: Alan Coopersmith +Reviewed-by: Peter Hutterer +--- + dix/dispatch.c | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/dix/dispatch.c b/dix/dispatch.c +index d844a09..55b978d 100644 +--- dix/dispatch.c ++++ dix/dispatch.c +@@ -2000,6 +2000,9 @@ ProcPutImage(ClientPtr client) + tmpImage = (char *) &stuff[1]; + lengthProto = length; + ++ if (lengthProto >= (INT32_MAX / stuff->height)) ++ return BadLength; ++ + if ((bytes_to_int32(lengthProto * stuff->height) + + bytes_to_int32(sizeof(xPutImageReq))) != client->req_len) + return BadLength; +-- +2.1.2 + Added: head/x11-servers/xorg-server/files/patch-CVE-2014-8092-2-4 ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/x11-servers/xorg-server/files/patch-CVE-2014-8092-2-4 Wed Dec 10 21:35:13 2014 (r374489) @@ -0,0 +1,49 @@ +From bc8e20430b6f6378daf6ce4329029248a88af08b Mon Sep 17 00:00:00 2001 +From: Alan Coopersmith +Date: Mon, 6 Jan 2014 23:30:14 -0800 +Subject: [PATCH 02/40] dix: integer overflow in GetHosts() [CVE-2014-8092 2/4] + +GetHosts() iterates over all the hosts it has in memory, and copies +them to a buffer. The buffer length is calculated by iterating over +all the hosts and adding up all of their combined length. There is a +potential integer overflow, if there are lots and lots of hosts (with +a combined length of > ~4 gig). This should be possible by repeatedly +calling ProcChangeHosts() on 64bit machines with enough memory. + +This patch caps the list at 1mb, because multi-megabyte hostname +lists for X access control are insane. + +Reported-by: Ilja Van Sprundel +Signed-off-by: Alan Coopersmith +Reviewed-by: Peter Hutterer +--- + os/access.c | 6 ++++++ + 1 file changed, 6 insertions(+) + +diff --git a/os/access.c b/os/access.c +index 5c510de..f393c8d 100644 +--- os/access.c ++++ os/access.c +@@ -1296,6 +1296,10 @@ GetHosts(void **data, int *pnHosts, int *pLen, BOOL * pEnabled) + for (host = validhosts; host; host = host->next) { + nHosts++; + n += pad_to_int32(host->len) + sizeof(xHostEntry); ++ /* Could check for INT_MAX, but in reality having more than 1mb of ++ hostnames in the access list is ridiculous */ ++ if (n >= 1048576) ++ break; + } + if (n) { + *data = ptr = malloc(n); +@@ -1304,6 +1308,8 @@ GetHosts(void **data, int *pnHosts, int *pLen, BOOL * pEnabled) + } + for (host = validhosts; host; host = host->next) { + len = host->len; ++ if ((ptr + sizeof(xHostEntry) + len) > (data + n)) ++ break; + ((xHostEntry *) ptr)->family = host->family; + ((xHostEntry *) ptr)->length = len; + ptr += sizeof(xHostEntry); +-- +2.1.2 + Added: head/x11-servers/xorg-server/files/patch-CVE-2014-8092-3-4 ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/x11-servers/xorg-server/files/patch-CVE-2014-8092-3-4 Wed Dec 10 21:35:13 2014 (r374489) @@ -0,0 +1,128 @@ +From 97015a07b9e15d8ec5608b95d95ec0eb51202acb Mon Sep 17 00:00:00 2001 +From: Alan Coopersmith +Date: Wed, 22 Jan 2014 22:37:15 -0800 +Subject: [PATCH 03/40] dix: integer overflow in RegionSizeof() [CVE-2014-8092 + 3/4] + +RegionSizeof contains several integer overflows if a large length +value is passed in. Once we fix it to return 0 on overflow, we +also have to fix the callers to handle this error condition + +v2: Fixed limit calculation in RegionSizeof as pointed out by jcristau. + +Reported-by: Ilja Van Sprundel +Signed-off-by: Alan Coopersmith +Reviewed-by: Peter Hutterer +Reviewed-by: Julien Cristau +--- + dix/region.c | 20 +++++++++++++------- + include/regionstr.h | 10 +++++++--- + 2 files changed, 20 insertions(+), 10 deletions(-) + +diff --git a/dix/region.c b/dix/region.c +index ce1014e..04e5901 100644 +--- dix/region.c ++++ dix/region.c +@@ -169,7 +169,6 @@ Equipment Corporation. + ((r1)->y1 <= (r2)->y1) && \ + ((r1)->y2 >= (r2)->y2) ) + +-#define xallocData(n) malloc(RegionSizeof(n)) + #define xfreeData(reg) if ((reg)->data && (reg)->data->size) free((reg)->data) + + #define RECTALLOC_BAIL(pReg,n,bail) \ +@@ -205,8 +204,9 @@ if (!(pReg)->data || (((pReg)->data->numRects + (n)) > (pReg)->data->size)) \ + #define DOWNSIZE(reg,numRects) \ + if (((numRects) < ((reg)->data->size >> 1)) && ((reg)->data->size > 50)) \ + { \ +- RegDataPtr NewData; \ +- NewData = (RegDataPtr)realloc((reg)->data, RegionSizeof(numRects)); \ ++ size_t NewSize = RegionSizeof(numRects); \ ++ RegDataPtr NewData = \ ++ (NewSize > 0) ? realloc((reg)->data, NewSize) : NULL ; \ + if (NewData) \ + { \ + NewData->size = (numRects); \ +@@ -345,17 +345,20 @@ Bool + RegionRectAlloc(RegionPtr pRgn, int n) + { + RegDataPtr data; ++ size_t rgnSize; + + if (!pRgn->data) { + n++; +- pRgn->data = xallocData(n); ++ rgnSize = RegionSizeof(n); ++ pRgn->data = (rgnSize > 0) ? malloc(rgnSize) : NULL; + if (!pRgn->data) + return RegionBreak(pRgn); + pRgn->data->numRects = 1; + *RegionBoxptr(pRgn) = pRgn->extents; + } + else if (!pRgn->data->size) { +- pRgn->data = xallocData(n); ++ rgnSize = RegionSizeof(n); ++ pRgn->data = (rgnSize > 0) ? malloc(rgnSize) : NULL; + if (!pRgn->data) + return RegionBreak(pRgn); + pRgn->data->numRects = 0; +@@ -367,7 +370,8 @@ RegionRectAlloc(RegionPtr pRgn, int n) + n = 250; + } + n += pRgn->data->numRects; +- data = (RegDataPtr) realloc(pRgn->data, RegionSizeof(n)); ++ rgnSize = RegionSizeof(n); ++ data = (rgnSize > 0) ? realloc(pRgn->data, rgnSize) : NULL; + if (!data) + return RegionBreak(pRgn); + pRgn->data = data; +@@ -1312,6 +1316,7 @@ RegionFromRects(int nrects, xRectangle *prect, int ctype) + { + + RegionPtr pRgn; ++ size_t rgnSize; + RegDataPtr pData; + BoxPtr pBox; + int i; +@@ -1338,7 +1343,8 @@ RegionFromRects(int nrects, xRectangle *prect, int ctype) + } + return pRgn; + } +- pData = xallocData(nrects); ++ rgnSize = RegionSizeof(nrects); ++ pData = (rgnSize > 0) ? malloc(rgnSize) : NULL; + if (!pData) { + RegionBreak(pRgn); + return pRgn; +diff --git a/include/regionstr.h b/include/regionstr.h +index 515e93f..079375d 100644 +--- include/regionstr.h ++++ include/regionstr.h +@@ -127,7 +127,10 @@ RegionEnd(RegionPtr reg) + static inline size_t + RegionSizeof(size_t n) + { +- return (sizeof(RegDataRec) + ((n) * sizeof(BoxRec))); ++ if (n < ((INT_MAX - sizeof(RegDataRec)) / sizeof(BoxRec))) ++ return (sizeof(RegDataRec) + ((n) * sizeof(BoxRec))); ++ else ++ return 0; + } + + static inline void +@@ -138,9 +141,10 @@ RegionInit(RegionPtr _pReg, BoxPtr _rect, int _size) + (_pReg)->data = (RegDataPtr) NULL; + } + else { ++ size_t rgnSize; + (_pReg)->extents = RegionEmptyBox; +- if (((_size) > 1) && ((_pReg)->data = +- (RegDataPtr) malloc(RegionSizeof(_size)))) { ++ if (((_size) > 1) && ((rgnSize = RegionSizeof(_size)) > 0) && ++ (((_pReg)->data = malloc(rgnSize)) != NULL)) { + (_pReg)->data->size = (_size); + (_pReg)->data->numRects = 0; + } +-- +2.1.2 + Added: head/x11-servers/xorg-server/files/patch-CVE-2014-8092-4-4 ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/x11-servers/xorg-server/files/patch-CVE-2014-8092-4-4 Wed Dec 10 21:35:13 2014 (r374489) @@ -0,0 +1,34 @@ +From e0e11644622a589129a01e11e5d105dc74a098de Mon Sep 17 00:00:00 2001 +From: Alan Coopersmith +Date: Wed, 22 Jan 2014 23:44:46 -0800 +Subject: [PATCH 04/40] dix: integer overflow in REQUEST_FIXED_SIZE() + [CVE-2014-8092 4/4] + +Force use of 64-bit integers when evaluating data provided by clients +in 32-bit fields which can overflow when added or multiplied during +checks. + +Reported-by: Ilja Van Sprundel +Signed-off-by: Alan Coopersmith +Reviewed-by: Peter Hutterer +--- + include/dix.h | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/include/dix.h b/include/dix.h +index 991a3ce..e0c6ed8 100644 +--- include/dix.h ++++ include/dix.h +@@ -76,7 +76,8 @@ SOFTWARE. + + #define REQUEST_FIXED_SIZE(req, n)\ + if (((sizeof(req) >> 2) > client->req_len) || \ +- (((sizeof(req) + (n) + 3) >> 2) != client->req_len)) \ ++ ((n >> 2) >= client->req_len) || \ ++ ((((uint64_t) sizeof(req) + (n) + 3) >> 2) != (uint64_t) client->req_len)) \ + return(BadLength) + + #define LEGAL_NEW_RESOURCE(id,client)\ +-- +2.1.2 + Added: head/x11-servers/xorg-server/files/patch-CVE-2014-8092-pt5 ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/x11-servers/xorg-server/files/patch-CVE-2014-8092-pt5 Wed Dec 10 21:35:13 2014 (r374489) @@ -0,0 +1,32 @@ +From 9802a0162f738de03585ca3f3b8a8266494f7d45 Mon Sep 17 00:00:00 2001 +From: Keith Packard +Date: Tue, 9 Dec 2014 09:30:59 -0800 +Subject: [PATCH 38/40] Missing parens in REQUEST_FIXED_SIZE macro + [CVE-2014-8092 pt. 5] + +The 'n' parameter must be surrounded by parens in both places to +prevent precedence from mis-computing things. + +Signed-off-by: Keith Packard +Reviewed-by: Alan Coopersmith +Signed-off-by: Alan Coopersmith +--- + include/dix.h | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/include/dix.h b/include/dix.h +index 21176a8..921156b 100644 +--- include/dix.h ++++ include/dix.h +@@ -80,7 +80,7 @@ SOFTWARE. + + #define REQUEST_FIXED_SIZE(req, n)\ + if (((sizeof(req) >> 2) > client->req_len) || \ +- ((n >> 2) >= client->req_len) || \ ++ (((n) >> 2) >= client->req_len) || \ + ((((uint64_t) sizeof(req) + (n) + 3) >> 2) != (uint64_t) client->req_len)) \ + return(BadLength) + +-- +2.1.2 + Added: head/x11-servers/xorg-server/files/patch-CVE-2014-8092-pt6 ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/x11-servers/xorg-server/files/patch-CVE-2014-8092-pt6 Wed Dec 10 21:35:13 2014 (r374489) @@ -0,0 +1,35 @@ +From 1559a94395258fd73e369f1a2c98a44bfe21a486 Mon Sep 17 00:00:00 2001 +From: Keith Packard +Date: Tue, 9 Dec 2014 09:31:00 -0800 +Subject: [PATCH 39/40] dix: GetHosts bounds check using wrong pointer value + [CVE-2014-8092 pt. 6] + +GetHosts saves the pointer to allocated memory in *data, and then +wants to bounds-check writes to that region, but was mistakenly using +a bare 'data' instead of '*data'. Also, data is declared as void **, +so we need a cast to turn it into a byte pointer so we can actually do +pointer comparisons. + +Signed-off-by: Keith Packard +Reviewed-by: Alan Coopersmith +Signed-off-by: Alan Coopersmith +--- + os/access.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/os/access.c b/os/access.c +index f393c8d..28f2d32 100644 +--- os/access.c ++++ os/access.c +@@ -1308,7 +1308,7 @@ GetHosts(void **data, int *pnHosts, int *pLen, BOOL * pEnabled) + } + for (host = validhosts; host; host = host->next) { + len = host->len; +- if ((ptr + sizeof(xHostEntry) + len) > (data + n)) ++ if ((ptr + sizeof(xHostEntry) + len) > ((unsigned char *) *data + n)) + break; + ((xHostEntry *) ptr)->family = host->family; + ((xHostEntry *) ptr)->length = len; +-- +2.1.2 + Added: head/x11-servers/xorg-server/files/patch-CVE-2014-8093-1-6 ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/x11-servers/xorg-server/files/patch-CVE-2014-8093-1-6 Wed Dec 10 21:35:13 2014 (r374489) @@ -0,0 +1,47 @@ +From 23fe7718bb171e71db2d1a30505c2ca2988799d9 Mon Sep 17 00:00:00 2001 +From: Adam Jackson +Date: Mon, 10 Nov 2014 12:13:36 -0500 +Subject: [PATCH 19/40] glx: Be more paranoid about variable-length requests + [CVE-2014-8093 1/6] + +If the size computation routine returns -1 we should just reject the +request outright. Clamping it to zero could give an attacker the +opportunity to also mangle cmdlen in such a way that the subsequent +length check passes, and the request would get executed, thus passing +data we wanted to reject to the renderer. + +Reviewed-by: Keith Packard +Reviewed-by: Julien Cristau +Reviewed-by: Michal Srb +Reviewed-by: Andy Ritger +Signed-off-by: Adam Jackson +Signed-off-by: Alan Coopersmith +--- + glx/glxcmds.c | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/glx/glxcmds.c b/glx/glxcmds.c +index 009fd9b..ea42e2a 100644 +--- glx/glxcmds.c ++++ glx/glxcmds.c +@@ -2062,7 +2062,7 @@ __glXDisp_Render(__GLXclientState * cl, GLbyte * pc) + extra = (*entry.varsize) (pc + __GLX_RENDER_HDR_SIZE, + client->swapped); + if (extra < 0) { +- extra = 0; ++ return BadLength; + } + if (cmdlen != __GLX_PAD(entry.bytes + extra)) { + return BadLength; +@@ -2179,7 +2179,7 @@ __glXDisp_RenderLarge(__GLXclientState * cl, GLbyte * pc) + extra = (*entry.varsize) (pc + __GLX_RENDER_LARGE_HDR_SIZE, + client->swapped); + if (extra < 0) { +- extra = 0; ++ return BadLength; + } + /* large command's header is 4 bytes longer, so add 4 */ + if (cmdlen != __GLX_PAD(entry.bytes + 4 + extra)) { +-- +2.1.2 + Added: head/x11-servers/xorg-server/files/patch-CVE-2014-8093-2-6 ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/x11-servers/xorg-server/files/patch-CVE-2014-8093-2-6 Wed Dec 10 21:35:13 2014 (r374489) @@ -0,0 +1,165 @@ +From ab2ba9338aa5e85b4487bc7fbe69985c76483e01 Mon Sep 17 00:00:00 2001 +From: Adam Jackson +Date: Mon, 10 Nov 2014 12:13:37 -0500 +Subject: [PATCH 20/40] glx: Be more strict about rejecting invalid image sizes + [CVE-2014-8093 2/6] + +Before this we'd just clamp the image size to 0, which was just +hideously stupid; if the parameters were such that they'd overflow an +integer, you'd allocate a small buffer, then pass huge values into (say) +ReadPixels, and now you're scribbling over arbitrary server memory. + +Reviewed-by: Keith Packard +Reviewed-by: Julien Cristau +Reviewed-by: Michal Srb +Reviewed-by: Andy Ritger +Signed-off-by: Adam Jackson +Signed-off-by: Alan Coopersmith +--- + glx/singlepix.c | 16 ++++++++-------- + glx/singlepixswap.c | 16 ++++++++-------- + 2 files changed, 16 insertions(+), 16 deletions(-) + +diff --git a/glx/singlepix.c b/glx/singlepix.c +index 506fdaa..8b6c261 100644 +--- glx/singlepix.c ++++ glx/singlepix.c +@@ -65,7 +65,7 @@ __glXDisp_ReadPixels(__GLXclientState * cl, GLbyte * pc) + lsbFirst = *(GLboolean *) (pc + 25); + compsize = __glReadPixels_size(format, type, width, height); + if (compsize < 0) +- compsize = 0; ++ return BadLength; + + glPixelStorei(GL_PACK_SWAP_BYTES, swapBytes); + glPixelStorei(GL_PACK_LSB_FIRST, lsbFirst); +@@ -124,7 +124,7 @@ __glXDisp_GetTexImage(__GLXclientState * cl, GLbyte * pc) + compsize = + __glGetTexImage_size(target, level, format, type, width, height, depth); + if (compsize < 0) +- compsize = 0; ++ return BadLength; + + glPixelStorei(GL_PACK_SWAP_BYTES, swapBytes); + __GLX_GET_ANSWER_BUFFER(answer, cl, compsize, 1); +@@ -218,9 +218,9 @@ GetSeparableFilter(__GLXclientState * cl, GLbyte * pc, GLXContextTag tag) + compsize2 = __glGetTexImage_size(target, 1, format, type, height, 1, 1); + + if (compsize < 0) +- compsize = 0; ++ return BadLength; + if (compsize2 < 0) +- compsize2 = 0; ++ return BadLength; + compsize = __GLX_PAD(compsize); + compsize2 = __GLX_PAD(compsize2); + +@@ -296,7 +296,7 @@ GetConvolutionFilter(__GLXclientState * cl, GLbyte * pc, GLXContextTag tag) + */ + compsize = __glGetTexImage_size(target, 1, format, type, width, height, 1); + if (compsize < 0) +- compsize = 0; ++ return BadLength; + + glPixelStorei(GL_PACK_SWAP_BYTES, swapBytes); + __GLX_GET_ANSWER_BUFFER(answer, cl, compsize, 1); +@@ -365,7 +365,7 @@ GetHistogram(__GLXclientState * cl, GLbyte * pc, GLXContextTag tag) + */ + compsize = __glGetTexImage_size(target, 1, format, type, width, 1, 1); + if (compsize < 0) +- compsize = 0; ++ return BadLength; + + glPixelStorei(GL_PACK_SWAP_BYTES, swapBytes); + __GLX_GET_ANSWER_BUFFER(answer, cl, compsize, 1); +@@ -426,7 +426,7 @@ GetMinmax(__GLXclientState * cl, GLbyte * pc, GLXContextTag tag) + + compsize = __glGetTexImage_size(target, 1, format, type, 2, 1, 1); + if (compsize < 0) +- compsize = 0; ++ return BadLength; + + glPixelStorei(GL_PACK_SWAP_BYTES, swapBytes); + __GLX_GET_ANSWER_BUFFER(answer, cl, compsize, 1); +@@ -491,7 +491,7 @@ GetColorTable(__GLXclientState * cl, GLbyte * pc, GLXContextTag tag) + */ + compsize = __glGetTexImage_size(target, 1, format, type, width, 1, 1); + if (compsize < 0) +- compsize = 0; ++ return BadLength; + + glPixelStorei(GL_PACK_SWAP_BYTES, swapBytes); + __GLX_GET_ANSWER_BUFFER(answer, cl, compsize, 1); +diff --git a/glx/singlepixswap.c b/glx/singlepixswap.c +index 8469101..8dc304f 100644 +--- glx/singlepixswap.c ++++ glx/singlepixswap.c +@@ -75,7 +75,7 @@ __glXDispSwap_ReadPixels(__GLXclientState * cl, GLbyte * pc) + lsbFirst = *(GLboolean *) (pc + 25); + compsize = __glReadPixels_size(format, type, width, height); + if (compsize < 0) +- compsize = 0; ++ return BadLength; + + glPixelStorei(GL_PACK_SWAP_BYTES, !swapBytes); + glPixelStorei(GL_PACK_LSB_FIRST, lsbFirst); +@@ -144,7 +144,7 @@ __glXDispSwap_GetTexImage(__GLXclientState * cl, GLbyte * pc) + compsize = + __glGetTexImage_size(target, level, format, type, width, height, depth); + if (compsize < 0) +- compsize = 0; ++ return BadLength; + + glPixelStorei(GL_PACK_SWAP_BYTES, !swapBytes); + __GLX_GET_ANSWER_BUFFER(answer, cl, compsize, 1); +@@ -252,9 +252,9 @@ GetSeparableFilter(__GLXclientState * cl, GLbyte * pc, GLXContextTag tag) + compsize2 = __glGetTexImage_size(target, 1, format, type, height, 1, 1); + + if (compsize < 0) +- compsize = 0; ++ return BadLength; + if (compsize2 < 0) +- compsize2 = 0; ++ return BadLength; + compsize = __GLX_PAD(compsize); + compsize2 = __GLX_PAD(compsize2); + +@@ -338,7 +338,7 @@ GetConvolutionFilter(__GLXclientState * cl, GLbyte * pc, GLXContextTag tag) + */ + compsize = __glGetTexImage_size(target, 1, format, type, width, height, 1); + if (compsize < 0) +- compsize = 0; ++ return BadLength; + + glPixelStorei(GL_PACK_SWAP_BYTES, !swapBytes); + __GLX_GET_ANSWER_BUFFER(answer, cl, compsize, 1); +@@ -415,7 +415,7 @@ GetHistogram(__GLXclientState * cl, GLbyte * pc, GLXContextTag tag) + */ + compsize = __glGetTexImage_size(target, 1, format, type, width, 1, 1); + if (compsize < 0) +- compsize = 0; ++ return BadLength; + + glPixelStorei(GL_PACK_SWAP_BYTES, !swapBytes); + __GLX_GET_ANSWER_BUFFER(answer, cl, compsize, 1); +@@ -483,7 +483,7 @@ GetMinmax(__GLXclientState * cl, GLbyte * pc, GLXContextTag tag) + + compsize = __glGetTexImage_size(target, 1, format, type, 2, 1, 1); + if (compsize < 0) +- compsize = 0; ++ return BadLength; + + glPixelStorei(GL_PACK_SWAP_BYTES, !swapBytes); + __GLX_GET_ANSWER_BUFFER(answer, cl, compsize, 1); +@@ -554,7 +554,7 @@ GetColorTable(__GLXclientState * cl, GLbyte * pc, GLXContextTag tag) + */ + compsize = __glGetTexImage_size(target, 1, format, type, width, 1, 1); + if (compsize < 0) +- compsize = 0; ++ return BadLength; + + glPixelStorei(GL_PACK_SWAP_BYTES, !swapBytes); + __GLX_GET_ANSWER_BUFFER(answer, cl, compsize, 1); +-- +2.1.2 + Added: head/x11-servers/xorg-server/files/patch-CVE-2014-8093-3-6 ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/x11-servers/xorg-server/files/patch-CVE-2014-8093-3-6 Wed Dec 10 21:35:13 2014 (r374489) @@ -0,0 +1,59 @@ +From 717a1b37767b41e14859e5022ae9e679152821a9 Mon Sep 17 00:00:00 2001 +From: Adam Jackson +Date: Mon, 10 Nov 2014 12:13:38 -0500 +Subject: [PATCH 21/40] glx: Additional paranoia in __glXGetAnswerBuffer / + __GLX_GET_ANSWER_BUFFER (v2) [CVE-2014-8093 3/6] + +If the computed reply size is negative, something went wrong, treat it +as an error. + +v2: Be more careful about size_t being unsigned (Matthieu Herrb) +v3: SIZE_MAX not SIZE_T_MAX (Alan Coopersmith) + +Reviewed-by: Julien Cristau +Reviewed-by: Michal Srb +Reviewed-by: Andy Ritger +Signed-off-by: Adam Jackson +Signed-off-by: Alan Coopersmith +--- + glx/indirect_util.c | 7 ++++++- + glx/unpack.h | 3 ++- + 2 files changed, 8 insertions(+), 2 deletions(-) + +diff --git a/glx/indirect_util.c b/glx/indirect_util.c +index 926e57c..de81491 100644 +--- glx/indirect_util.c ++++ glx/indirect_util.c +@@ -76,9 +76,14 @@ __glXGetAnswerBuffer(__GLXclientState * cl, size_t required_size, + const unsigned mask = alignment - 1; + + if (local_size < required_size) { +- const size_t worst_case_size = required_size + alignment; ++ size_t worst_case_size; + intptr_t temp_buf; + ++ if (required_size < SIZE_MAX - alignment) ++ worst_case_size = required_size + alignment; ++ else ++ return NULL; ++ + if (cl->returnBufSize < worst_case_size) { + void *temp = realloc(cl->returnBuf, worst_case_size); + +diff --git a/glx/unpack.h b/glx/unpack.h +index 52fba74..2b1ebcf 100644 +--- glx/unpack.h ++++ glx/unpack.h +@@ -83,7 +83,8 @@ extern xGLXSingleReply __glXReply; + ** pointer. + */ + #define __GLX_GET_ANSWER_BUFFER(res,cl,size,align) \ +- if ((size) > sizeof(answerBuffer)) { \ ++ if (size < 0) return BadLength; \ ++ else if ((size) > sizeof(answerBuffer)) { \ + int bump; \ + if ((cl)->returnBufSize < (size)+(align)) { \ + (cl)->returnBuf = (GLbyte*)realloc((cl)->returnBuf, \ +-- +2.1.2 + Added: head/x11-servers/xorg-server/files/patch-CVE-2014-8093-4-6 ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/x11-servers/xorg-server/files/patch-CVE-2014-8093-4-6 Wed Dec 10 21:35:13 2014 (r374489) @@ -0,0 +1,82 @@ +From 2a5cbc17fc72185bf0fa06fef26d1f782de72595 Mon Sep 17 00:00:00 2001 +From: Adam Jackson +Date: Mon, 10 Nov 2014 12:13:40 -0500 +Subject: [PATCH 23/40] glx: Add safe_{add,mul,pad} (v3) [CVE-2014-8093 4/6] + +These are paranoid about integer overflow, and will return -1 if their +operation would overflow a (signed) integer or if either argument is +negative. + +Note that RenderLarge requests are sized with a uint32_t so in principle +this could be sketchy there, but dix limits bigreqs to 128M so you +shouldn't ever notice, and honestly if you're sending more than 2G of +rendering commands you're already doing something very wrong. + +v2: Use INT_MAX for consistency with the rest of the server (jcristau) +v3: Reject negative arguments (anholt) + +Reviewed-by: Keith Packard +Reviewed-by: Julien Cristau +Reviewed-by: Michal Srb +Reviewed-by: Andy Ritger +Signed-off-by: Adam Jackson +Signed-off-by: Alan Coopersmith +--- + glx/glxserver.h | 41 +++++++++++++++++++++++++++++++++++++++++ + 1 file changed, 41 insertions(+) + +diff --git a/glx/glxserver.h b/glx/glxserver.h +index a324b29..9482601 100644 +--- glx/glxserver.h ++++ glx/glxserver.h +@@ -228,6 +228,47 @@ extern void glxSwapQueryServerStringReply(ClientPtr client, + * Routines for computing the size of variably-sized rendering commands. + */ + ++static _X_INLINE int ++safe_add(int a, int b) ++{ ++ if (a < 0 || b < 0) ++ return -1; ++ ++ if (INT_MAX - a < b) ++ return -1; ++ ++ return a + b; ++} ++ ++static _X_INLINE int ++safe_mul(int a, int b) ++{ ++ if (a < 0 || b < 0) ++ return -1; ++ ++ if (a == 0 || b == 0) ++ return 0; ++ ++ if (a > INT_MAX / b) ++ return -1; ++ ++ return a * b; ++} ++ ++static _X_INLINE int ++safe_pad(int a) ++{ ++ int ret; ++ ++ if (a < 0) ++ return -1; ++ ++ if ((ret = safe_add(a, 3)) < 0) ++ return -1; ++ ++ return ret & (GLuint)~3; ++} ++ + extern int __glXTypeSize(GLenum enm); + extern int __glXImageSize(GLenum format, GLenum type, + GLenum target, GLsizei w, GLsizei h, GLsizei d, +-- +2.1.2 + Added: head/x11-servers/xorg-server/files/patch-CVE-2014-8093-5-6 ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/x11-servers/xorg-server/files/patch-CVE-2014-8093-5-6 Wed Dec 10 21:35:13 2014 (r374489) @@ -0,0 +1,225 @@ +From 698888e6671d54c7ae41e9d456f7f5483a3459d2 Mon Sep 17 00:00:00 2001 +From: Adam Jackson +Date: Mon, 10 Nov 2014 12:13:42 -0500 +Subject: [PATCH 25/40] glx: Integer overflow protection for non-generated + render requests (v3) [CVE-2014-8093 5/6] + +v2: +Fix constants in __glXMap2fReqSize (Michal Srb) +Validate w/h/d for proxy targets too (Keith Packard) + +v3: +Fix Map[12]Size to correctly reject order == 0 (Julien Cristau) + +Reviewed-by: Keith Packard +Reviewed-by: Michal Srb +Reviewed-by: Andy Ritger +Signed-off-by: Adam Jackson +Signed-off-by: Alan Coopersmith +--- + glx/rensize.c | 77 +++++++++++++++++++++++++++++++---------------------------- + 1 file changed, 41 insertions(+), 36 deletions(-) + +diff --git a/glx/rensize.c b/glx/rensize.c +index 9ff73c7..d46334a 100644 +--- glx/rensize.c ++++ glx/rensize.c +@@ -43,19 +43,11 @@ + (((a & 0xff000000U)>>24) | ((a & 0xff0000U)>>8) | \ + ((a & 0xff00U)<<8) | ((a & 0xffU)<<24)) + +-static int +-Map1Size(GLint k, GLint order) +-{ +- if (order <= 0 || k < 0) +- return -1; +- return k * order; +-} +- + int + __glXMap1dReqSize(const GLbyte * pc, Bool swap) + { + GLenum target; +- GLint order, k; ++ GLint order; + + target = *(GLenum *) (pc + 16); + order = *(GLint *) (pc + 20); +@@ -63,15 +55,16 @@ __glXMap1dReqSize(const GLbyte * pc, Bool swap) + target = SWAPL(target); + order = SWAPL(order); + } +- k = __glMap1d_size(target); +- return 8 * Map1Size(k, order); ++ if (order < 1) ++ return -1; ++ return safe_mul(8, safe_mul(__glMap1d_size(target), order)); + } + + int + __glXMap1fReqSize(const GLbyte * pc, Bool swap) + { + GLenum target; +- GLint order, k; ++ GLint order; + + target = *(GLenum *) (pc + 0); + order = *(GLint *) (pc + 12); +@@ -79,23 +72,24 @@ __glXMap1fReqSize(const GLbyte * pc, Bool swap) + target = SWAPL(target); + order = SWAPL(order); + } +- k = __glMap1f_size(target); +- return 4 * Map1Size(k, order); ++ if (order < 1) ++ return -1; ++ return safe_mul(4, safe_mul(__glMap1f_size(target), order)); + } + + static int + Map2Size(int k, int majorOrder, int minorOrder) + { +- if (majorOrder <= 0 || minorOrder <= 0 || k < 0) ++ if (majorOrder < 1 || minorOrder < 1) + return -1; +- return k * majorOrder * minorOrder; ++ return safe_mul(k, safe_mul(majorOrder, minorOrder)); + } + + int + __glXMap2dReqSize(const GLbyte * pc, Bool swap) + { + GLenum target; +- GLint uorder, vorder, k; ++ GLint uorder, vorder; + + target = *(GLenum *) (pc + 32); + uorder = *(GLint *) (pc + 36); +@@ -105,15 +99,14 @@ __glXMap2dReqSize(const GLbyte * pc, Bool swap) + uorder = SWAPL(uorder); + vorder = SWAPL(vorder); + } +- k = __glMap2d_size(target); +- return 8 * Map2Size(k, uorder, vorder); ++ return safe_mul(8, Map2Size(__glMap2d_size(target), uorder, vorder)); + } + + int + __glXMap2fReqSize(const GLbyte * pc, Bool swap) + { + GLenum target; +- GLint uorder, vorder, k; ++ GLint uorder, vorder; + + target = *(GLenum *) (pc + 0); + uorder = *(GLint *) (pc + 12); +@@ -123,8 +116,7 @@ __glXMap2fReqSize(const GLbyte * pc, Bool swap) + uorder = SWAPL(uorder); + vorder = SWAPL(vorder); + } +- k = __glMap2f_size(target); +- return 4 * Map2Size(k, uorder, vorder); ++ return safe_mul(4, Map2Size(__glMap2f_size(target), uorder, vorder)); + } + + /** +@@ -175,14 +167,16 @@ __glXImageSize(GLenum format, GLenum type, GLenum target, + GLint bytesPerElement, elementsPerGroup, groupsPerRow; + GLint groupSize, rowSize, padding, imageSize; + ++ if (w == 0 || h == 0 || d == 0) ++ return 0; ++ + if (w < 0 || h < 0 || d < 0 || + (type == GL_BITMAP && + (format != GL_COLOR_INDEX && format != GL_STENCIL_INDEX))) { + return -1; + } +- if (w == 0 || h == 0 || d == 0) +- return 0; + ++ /* proxy targets have no data */ + switch (target) { + case GL_PROXY_TEXTURE_1D: + case GL_PROXY_TEXTURE_2D: +@@ -199,6 +193,12 @@ __glXImageSize(GLenum format, GLenum type, GLenum target, + return 0; + } + ++ /* real data has to have real sizes */ ++ if (imageHeight < 0 || rowLength < 0 || skipImages < 0 || skipRows < 0) ++ return -1; ++ if (alignment != 1 && alignment != 2 && alignment != 4 && alignment != 8) ++ return -1; ++ + if (type == GL_BITMAP) { + if (rowLength > 0) { + groupsPerRow = rowLength; +@@ -207,11 +207,14 @@ __glXImageSize(GLenum format, GLenum type, GLenum target, + groupsPerRow = w; + } + rowSize = bits_to_bytes(groupsPerRow); ++ if (rowSize < 0) ++ return -1; + padding = (rowSize % alignment); + if (padding) { + rowSize += alignment - padding; + } +- return ((h + skipRows) * rowSize); ++ ++ return safe_mul(safe_add(h, skipRows), rowSize); + } + else { + switch (format) { +@@ -303,6 +306,7 @@ __glXImageSize(GLenum format, GLenum type, GLenum target, + default: + return -1; + } ++ /* known safe by the switches above, not checked */ + groupSize = bytesPerElement * elementsPerGroup; + if (rowLength > 0) { + groupsPerRow = rowLength; +@@ -310,18 +314,21 @@ __glXImageSize(GLenum format, GLenum type, GLenum target, + else { + groupsPerRow = w; + } +- rowSize = groupsPerRow * groupSize; ++ ++ if ((rowSize = safe_mul(groupsPerRow, groupSize)) < 0) ++ return -1; + padding = (rowSize % alignment); + if (padding) { + rowSize += alignment - padding; + } +- if (imageHeight > 0) { +- imageSize = (imageHeight + skipRows) * rowSize; +- } +- else { +- imageSize = (h + skipRows) * rowSize; +- } +- return ((d + skipImages) * imageSize); ++ ++ if (imageHeight > 0) ++ h = imageHeight; ++ h = safe_add(h, skipRows); ++ ++ imageSize = safe_mul(h, rowSize); ++ ++ return safe_mul(safe_add(d, skipImages), imageSize); + } + } + +@@ -445,9 +452,7 @@ __glXSeparableFilter2DReqSize(const GLbyte * pc, Bool swap) + /* XXX Should rowLength be used for either or both image? */ + image1size = __glXImageSize(format, type, 0, w, 1, 1, + 0, rowLength, 0, 0, alignment); +- image1size = __GLX_PAD(image1size); + image2size = __glXImageSize(format, type, 0, h, 1, 1, + 0, rowLength, 0, 0, alignment); +- return image1size + image2size; +- ++ return safe_add(safe_pad(image1size), image2size); + } +-- +2.1.2 + Added: head/x11-servers/xorg-server/files/patch-CVE-2014-8093-6-6 ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/x11-servers/xorg-server/files/patch-CVE-2014-8093-6-6 Wed Dec 10 21:35:13 2014 (r374489) @@ -0,0 +1,34 @@ +From 7e7630bbb775573eea2a2335adb9d190c3e1e971 Mon Sep 17 00:00:00 2001 +From: Robert Morell +Date: Wed, 12 Nov 2014 18:51:43 -0800 +Subject: [PATCH 32/40] glx: Fix mask truncation in __glXGetAnswerBuffer + [CVE-2014-8093 6/6] + +On a system where sizeof(unsigned) != sizeof(intptr_t), the unary +bitwise not operation will result in a mask that clears all high bits +from temp_buf in the expression: + temp_buf = (temp_buf + mask) & ~mask; + +Signed-off-by: Robert Morell +Reviewed-by: Alan Coopersmith +Signed-off-by: Alan Coopersmith +--- + glx/indirect_util.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/glx/indirect_util.c b/glx/indirect_util.c +index de81491..9ba2815 100644 +--- glx/indirect_util.c ++++ glx/indirect_util.c +@@ -73,7 +73,7 @@ __glXGetAnswerBuffer(__GLXclientState * cl, size_t required_size, + void *local_buffer, size_t local_size, unsigned alignment) + { + void *buffer = local_buffer; +- const unsigned mask = alignment - 1; ++ const intptr_t mask = alignment - 1; + + if (local_size < required_size) { + size_t worst_case_size; +-- +2.1.2 + Added: head/x11-servers/xorg-server/files/patch-CVE-2014-8094 ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/x11-servers/xorg-server/files/patch-CVE-2014-8094 Wed Dec 10 21:35:13 2014 (r374489) @@ -0,0 +1,35 @@ +From 6692670fde081bbfe9313f17d84037ae9116702a Mon Sep 17 00:00:00 2001 +From: Alan Coopersmith +Date: Wed, 22 Jan 2014 23:40:18 -0800 +Subject: [PATCH 05/40] dri2: integer overflow in ProcDRI2GetBuffers() + [CVE-2014-8094] + +ProcDRI2GetBuffers() tries to validate a length field (count). +There is an integer overflow in the validation. This can cause +out of bound reads and memory corruption later on. + +Reported-by: Ilja Van Sprundel +Signed-off-by: Alan Coopersmith +Reviewed-by: Peter Hutterer +Reviewed-by: Julien Cristau +--- + hw/xfree86/dri2/dri2ext.c | 3 +++ *** DIFF OUTPUT TRUNCATED AT 1000 LINES ***