Date: Sat, 23 Sep 2000 18:07:10 -0700 (PDT) From: Kris Kennaway <kris@FreeBSD.org> To: ports@freebsd.org Cc: jmz@freebsd.org, security@freebsd.org Subject: XFree86 3.x DoS patch Message-ID: <Pine.BSF.4.21.0009231756420.99058-200000@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
Please test this patch for the XFree86 port - it addresses a number of
denial of service/crash conditions in the X code (libraries used by client
programs, as well as parts of the server itself). These were reported on
bugtraq several months ago but never publically acknowledged by the
XFree86 developers, nor did they release a patch against 3.x. They did
however fix the problem silently in XFree86 4.x - this patch comes from
OpenBSD who tracked down and extracted the relevant patches from the
XFree86 CVS repo.
I'm not really sure of the impact of the vulnerabilities, but I think they
allow users who can connect remotely to an X application using the
vulnerable libraries to crash it without authenticating. Bad.
Kris
--
In God we Trust -- all others must submit an X.509 certificate.
-- Charles Forsythe <forsythe@alum.mit.edu>
[-- Attachment #2 --]
Some problems were discovered in X11 libraries which can cause DoS in
libICE and xdm. Also some potiential buffer overflow may occur in XKB
options parsing (although they can't be exploited in OpenBSD's default
setup where the X servers are not setuid). This patch fixes all these
problems:
Apply by doing:
cd "the directory containing your X11 source dir"
patch -p0 < 021_X11_libs.patch
And then rebuild your X11 tree:
cd X11
make all
make install
Index: lib/ICE/ICElibint.h
diff -u lib/ICE/ICElibint.h:1.1 X11/xc/lib/ICE/ICElibint.h:1.2
--- lib/ICE/ICElibint.h:1.1 Fri Sep 5 02:58:32 1997
+++ lib/ICE/ICElibint.h Mon Jul 10 15:17:09 2000
@@ -288,20 +288,21 @@
}
-#define SKIP_STRING(_pBuf, _swap) \
+#define SKIP_STRING(_pBuf, _swap, _end, _bail) \
{ \
CARD16 _len; \
EXTRACT_CARD16 (_pBuf, _swap, _len); \
- _pBuf += _len; \
- if (PAD32 (2 + _len)) \
- _pBuf += PAD32 (2 + _len); \
-}
+ _pBuf += _len + PAD32(2+_len); \
+ if (_pBuf > _end) { \
+ _bail; \
+ } \
+}
-#define SKIP_LISTOF_STRING(_pBuf, _swap, _count) \
+#define SKIP_LISTOF_STRING(_pBuf, _swap, _count, _end, _bail) \
{ \
int _i; \
for (_i = 0; _i < _count; _i++) \
- SKIP_STRING (_pBuf, _swap); \
+ SKIP_STRING (_pBuf, _swap, _end, _bail); \
}
Index: lib/ICE/process.c
diff -u lib/ICE/process.c:1.1 X11/xc/lib/ICE/process.c:1.2
--- lib/ICE/process.c:1.1 Fri Sep 5 02:58:32 1997
+++ lib/ICE/process.c Mon Jul 10 15:17:10 2000
@@ -63,7 +63,11 @@
return (0); \
}
-
+#define BAIL_STRING(_iceConn, _opcode, _pStart) {\
+ _IceErrorBadLength (_iceConn, 0, _opcode, IceFatalToConnection);\
+ IceDisposeCompleteMessage (_iceConn, _pStart);\
+ return (0);\
+}
/*
* IceProcessMessages:
@@ -819,7 +823,7 @@
int myAuthCount, hisAuthCount;
int found, i, j;
char *myAuthName, **hisAuthNames;
- char *pData, *pStart;
+ char *pData, *pStart, *pEnd;
char *vendor = NULL;
char *release = NULL;
int myAuthIndex = 0;
@@ -843,10 +847,18 @@
}
pData = pStart;
-
- SKIP_STRING (pData, swap); /* vendor */
- SKIP_STRING (pData, swap); /* release */
- SKIP_LISTOF_STRING (pData, swap, (int) message->authCount);/* auth names */
+ pEnd = pStart + (length << 3);
+
+ SKIP_STRING (pData, swap, pEnd,
+ BAIL_STRING(iceConn, ICE_ConnectionSetup,
+ pStart)); /* vendor */
+ SKIP_STRING (pData, swap, pEnd,
+ BAIL_STRING(iceConn, ICE_ConnectionSetup,
+ pStart)); /* release */
+ SKIP_LISTOF_STRING (pData, swap, (int) message->authCount, pEnd,
+ BAIL_STRING(iceConn, ICE_ConnectionSetup,
+ pStart)); /* auth names */
+
pData += (message->versionCount * 4); /* versions */
CHECK_COMPLETE_SIZE (iceConn, ICE_ConnectionSetup,
@@ -1685,7 +1697,7 @@
{
iceConnectionReplyMsg *message;
- char *pData, *pStart;
+ char *pData, *pStart, *pEnd;
Bool replyReady;
CHECK_AT_LEAST_SIZE (iceConn, ICE_ConnectionReply,
@@ -1701,9 +1713,14 @@
}
pData = pStart;
+ pEnd = pStart + (length << 3);
- SKIP_STRING (pData, swap); /* vendor */
- SKIP_STRING (pData, swap); /* release */
+ SKIP_STRING (pData, swap, pEnd,
+ BAIL_STRING (iceConn, ICE_ConnectionReply,
+ pStart)); /* vendor */
+ SKIP_STRING (pData, swap, pEnd,
+ BAIL_STRING (iceConn, ICE_ConnectionReply,
+ pStart)); /* release */
CHECK_COMPLETE_SIZE (iceConn, ICE_ConnectionReply,
length, pData - pStart + SIZEOF (iceConnectionReplyMsg),
@@ -1789,7 +1806,7 @@
int found, i, j;
char *myAuthName, **hisAuthNames;
char *protocolName;
- char *pData, *pStart;
+ char *pData, *pStart, *pEnd;
char *vendor = NULL;
char *release = NULL;
int accept_setup_now = 0;
@@ -1824,11 +1841,20 @@
}
pData = pStart;
+ pEnd = pStart + (length << 3);
- SKIP_STRING (pData, swap); /* proto name */
- SKIP_STRING (pData, swap); /* vendor */
- SKIP_STRING (pData, swap); /* release */
- SKIP_LISTOF_STRING (pData, swap, (int) message->authCount);/* auth names */
+ SKIP_STRING (pData, swap, pEnd,
+ BAIL_STRING(iceConn, ICE_ProtocolSetup,
+ pStart)); /* proto name */
+ SKIP_STRING (pData, swap, pEnd,
+ BAIL_STRING(iceConn, ICE_ProtocolSetup,
+ pStart)); /* vendor */
+ SKIP_STRING (pData, swap, pEnd,
+ BAIL_STRING(iceConn, ICE_ProtocolSetup,
+ pStart)); /* release */
+ SKIP_LISTOF_STRING (pData, swap, (int) message->authCount, pEnd,
+ BAIL_STRING(iceConn, ICE_ProtocolSetup,
+ pStart)); /* auth names */
pData += (message->versionCount * 4); /* versions */
CHECK_COMPLETE_SIZE (iceConn, ICE_ProtocolSetup,
@@ -2170,7 +2196,7 @@
{
iceProtocolReplyMsg *message;
- char *pData, *pStart;
+ char *pData, *pStart, *pEnd;
Bool replyReady;
CHECK_AT_LEAST_SIZE (iceConn, ICE_ProtocolReply,
@@ -2186,9 +2212,14 @@
}
pData = pStart;
+ pEnd = pStart + (length << 3);
- SKIP_STRING (pData, swap); /* vendor */
- SKIP_STRING (pData, swap); /* release */
+ SKIP_STRING (pData, swap, pEnd,
+ BAIL_STRING(iceConn, ICE_ProtocolReply,
+ pStart)); /* vendor */
+ SKIP_STRING (pData, swap, pEnd,
+ BAIL_STRING(iceConn, ICE_ProtocolReply,
+ pStart)); /* release */
CHECK_COMPLETE_SIZE (iceConn, ICE_ProtocolReply,
length, pData - pStart + SIZEOF (iceProtocolReplyMsg),
Index: lib/X11/GetProp.c
diff -u lib/X11/GetProp.c:1.1 X11/xc/lib/X11/GetProp.c:1.2
--- lib/X11/GetProp.c:1.1 Fri Sep 5 02:58:44 1997
+++ lib/X11/GetProp.c Mon Jul 10 15:20:35 2000
@@ -76,21 +76,24 @@
*/
case 8:
nbytes = netbytes = reply.nItems;
- if (*prop = (unsigned char *) Xmalloc ((unsigned)nbytes + 1))
+ if (nbytes + 1 > 0 &&
+ (*prop = (unsigned char *) Xmalloc ((unsigned)nbytes + 1)))
_XReadPad (dpy, (char *) *prop, netbytes);
break;
case 16:
nbytes = reply.nItems * sizeof (short);
netbytes = reply.nItems << 1;
- if (*prop = (unsigned char *) Xmalloc ((unsigned)nbytes + 1))
+ if (nbytes + 1 > 0 &&
+ (*prop = (unsigned char *) Xmalloc ((unsigned)nbytes + 1)))
_XRead16Pad (dpy, (short *) *prop, netbytes);
break;
case 32:
nbytes = reply.nItems * sizeof (long);
netbytes = reply.nItems << 2;
- if (*prop = (unsigned char *) Xmalloc ((unsigned)nbytes + 1))
+ if (nbytes + 1 > 0 &&
+ (*prop = (unsigned char *) Xmalloc ((unsigned)nbytes + 1)))
_XRead32 (dpy, (long *) *prop, netbytes);
break;
Index: lib/X11/OpenDis.c
diff -u lib/X11/OpenDis.c:1.1 X11/xc/lib/X11/OpenDis.c:1.2
--- lib/X11/OpenDis.c:1.1 Fri Sep 5 02:58:48 1997
+++ lib/X11/OpenDis.c Mon Jul 10 15:20:35 2000
@@ -371,6 +371,14 @@
dpy->max_request_size = u.setup->maxRequestSize;
mask = dpy->resource_mask;
dpy->resource_shift = 0;
+ if (!mask)
+ {
+ fprintf (stderr, "Xlib: connection to \"%s\" invalid setup\n",
+ fullname);
+ OutOfMemory(dpy, setup);
+ return (NULL);
+ }
+
while (!(mask & 1)) {
dpy->resource_shift++;
mask = mask >> 1;
@@ -390,6 +398,13 @@
(void) strncpy(dpy->vendor, u.vendor, vendorlen);
dpy->vendor[vendorlen] = '\0';
vendorlen = (vendorlen + 3) & ~3; /* round up */
+/*
+ * validate setup length
+ */
+ if ((int) setuplength - sz_xConnSetup - vendorlen < 0) {
+ OutOfMemory(dpy, setup);
+ return (NULL);
+ }
memmove (setup, u.vendor + vendorlen,
(int) setuplength - sz_xConnSetup - vendorlen);
u.vendor = setup;
@@ -568,6 +583,8 @@
if (_XReply (dpy, (xReply *) &reply, 0, xFalse)) {
if (reply.format == 8 && reply.propertyType == XA_STRING &&
+ (reply.nItems + 1 > 0) &&
+ (reply.nItems <= req->longLength * 4) &&
(dpy->xdefaults = Xmalloc (reply.nItems + 1))) {
_XReadPad (dpy, dpy->xdefaults, reply.nItems);
dpy->xdefaults[reply.nItems] = '\0';
Index: lib/X11/XlibInt.c
diff -u lib/X11/XlibInt.c:1.3 X11/xc/lib/X11/XlibInt.c:1.4
--- lib/X11/XlibInt.c:1.3 Tue Aug 24 12:11:19 1999
+++ lib/X11/XlibInt.c Mon Jul 10 15:20:35 2000
@@ -38,6 +38,8 @@
#define NEED_EVENTS
#define NEED_REPLIES
+#define GENERIC_LENGTH_LIMIT (1 << 29)
+
#include "Xlibint.h"
#include <X11/Xpoll.h>
#include <X11/Xtrans.h>
@@ -1689,6 +1691,17 @@
!= (char *)rep)
continue;
}
+ /*
+ * Don't accept ridiculously large values for
+ * generic.length; doing so could cause stack-scribbling
+ * problems elsewhere.
+ */
+ if (rep->generic.length > GENERIC_LENGTH_LIMIT) {
+ rep->generic.length = GENERIC_LENGTH_LIMIT;
+ (void) fprintf(stderr,
+ "Xlib: suspiciously long reply length %d set to %d",
+ rep->generic.length, GENERIC_LENGTH_LIMIT);
+ }
if (extra <= rep->generic.length) {
if (extra > 0)
/*
@@ -1827,6 +1840,13 @@
#endif
if (len > *lenp)
_XEatData(dpy, len - *lenp);
+ }
+ if (len < SIZEOF(xReply))
+ {
+ _XIOError (dpy);
+ buf += *lenp;
+ *lenp = 0;
+ return buf;
}
if (len >= *lenp) {
buf += *lenp;
Index: programs/Xserver/os/secauth.c
diff -u programs/Xserver/os/secauth.c:1.1 X11/xc/programs/Xserver/os/secauth.c:1.3
--- programs/Xserver/os/secauth.c:1.1 Fri Sep 5 03:15:14 1997
+++ programs/Xserver/os/secauth.c Mon Jul 10 15:23:26 2000
@@ -47,7 +47,7 @@
ClientPtr client;
char **reason;
{
- char *policy = *dataP;
+ CARD8 *policy = *(CARD8 **)dataP;
int length;
Bool permit;
int nPolicies;
@@ -61,13 +61,13 @@
}
permit = (*policy++ == 0);
- nPolicies = *policy++;
+ nPolicies = (CARD8) *policy++;
length -= 2;
sitePolicies = SecurityGetSitePolicyStrings(&nSitePolicies);
- while (nPolicies) {
+ while (nPolicies > 0) {
int strLen, sitePolicy;
if (length == 0) {
@@ -75,7 +75,7 @@
return FALSE;
}
- strLen = *policy++;
+ strLen = (CARD8) *policy++;
if (--length < strLen) {
*reason = InvalidPolicyReason;
return FALSE;
@@ -87,7 +87,7 @@
{
char *testPolicy = sitePolicies[sitePolicy];
if ((strLen == strlen(testPolicy)) &&
- (strncmp(policy, testPolicy, strLen) == 0))
+ (strncmp((char *)policy, testPolicy, strLen) == 0))
{
found = TRUE; /* need to continue parsing the policy... */
break;
@@ -107,7 +107,7 @@
}
*data_lengthP = length;
- *dataP = policy;
+ *dataP = (char *)policy;
return TRUE;
}
Index: programs/Xserver/os/xdmcp.c
diff -u programs/Xserver/os/xdmcp.c:1.1.1.2 X11/xc/programs/Xserver/os/xdmcp.c:1.2
--- programs/Xserver/os/xdmcp.c:1.1.1.2 Fri Jan 8 10:56:48 1999
+++ programs/Xserver/os/xdmcp.c Mon Jul 10 15:26:07 2000
@@ -1,5 +1,5 @@
/* $XConsortium: xdmcp.c /main/34 1996/12/02 10:23:29 lehors $ */
-/* $XFree86: xc/programs/Xserver/os/xdmcp.c,v 3.9.2.1 1998/12/18 11:56:34 dawes Exp $ */
+/* $XFree86: xc/programs/Xserver/os/xdmcp.c,v 3.9.2.2 2000/02/08 20:32:12 dawes Exp $ */
/*
* Copyright 1989 Network Computing Devices, Inc., Mountain View, California.
*
@@ -290,7 +290,10 @@
return (i + 1);
}
if (strcmp(argv[i], "-port") == 0) {
- ++i;
+ if (++i == argc) {
+ ErrorF("Xserver: missing port number in command line\n");
+ exit(1);
+ }
xdm_udp_port = atoi(argv[i]);
return (i + 1);
}
@@ -300,18 +303,28 @@
}
if (strcmp(argv[i], "-class") == 0) {
++i;
+ if (++i == argc) {
+ ErrorF("Xserver: missing class name in command line\n");
+ exit(1);
+ }
defaultDisplayClass = argv[i];
return (i + 1);
}
#ifdef HASXDMAUTH
if (strcmp(argv[i], "-cookie") == 0) {
- ++i;
+ if (++i == argc) {
+ ErrorF("Xserver: missing cookie data in command line\n");
+ exit(1);
+ }
xdmAuthCookie = argv[i];
return (i + 1);
}
#endif
if (strcmp(argv[i], "-displayID") == 0) {
- ++i;
+ if (++i == argc) {
+ ErrorF("Xserver: missing displayID in command line\n");
+ exit(1);
+ }
XdmcpRegisterManufacturerDisplayID (argv[i], strlen (argv[i]));
return (i + 1);
}
Index: programs/Xserver/xkb/ddxLoad.c
diff -u programs/Xserver/xkb/ddxLoad.c:1.1.1.3 X11/xc/programs/Xserver/xkb/ddxLoad.c:1.2
--- programs/Xserver/xkb/ddxLoad.c:1.1.1.3 Sat Nov 28 01:49:13 1998
+++ programs/Xserver/xkb/ddxLoad.c Mon Jul 10 15:28:10 2000
@@ -24,7 +24,7 @@
THE USE OR PERFORMANCE OF THIS SOFTWARE.
********************************************************/
-/* $XFree86: xc/programs/Xserver/xkb/ddxLoad.c,v 3.19.2.3 1998/09/27 12:59:29 hohndel Exp $ */
+/* $XFree86: xc/programs/Xserver/xkb/ddxLoad.c,v 3.19.2.4 2000/06/15 23:24:07 dawes Exp $ */
#include <stdio.h>
#include <ctype.h>
@@ -139,10 +139,8 @@
+strlen(file)+strlen(xkm_output_dir)
+strlen(outFile)+53 > PATH_MAX)
{
-#ifdef DEBUG
ErrorF("compiler command for keymap (%s) exceeds max length\n",
names->keymap);
-#endif
return False;
}
#ifndef __EMX__
@@ -169,10 +167,8 @@
+strlen(file)+strlen(xkm_output_dir)
+strlen(outFile)+49 > PATH_MAX)
{
-#ifdef DEBUG
ErrorF("compiler command for keymap (%s) exceeds max length\n",
names->keymap);
-#endif
return False;
}
sprintf(cmd,"xkbcomp -w %d -xkm %s%s -em1 %s -emp %s -eml %s keymap/%s %s%s.xkm",
@@ -236,6 +232,10 @@
sprintf(keymap,"server-%s",display);
}
else {
+ if (strlen(names->keymap) > PATH_MAX - 1) {
+ ErrorF("name of keymap (%s) exceeds max length\n", names->keymap);
+ return False;
+ }
strcpy(keymap,names->keymap);
}
@@ -254,10 +254,8 @@
+strlen(POST_ERROR_MSG1)+strlen(xkm_output_dir)
+strlen(keymap)+48 > PATH_MAX)
{
-#ifdef DEBUG
ErrorF("compiler command for keymap (%s) exceeds max length\n",
names->keymap);
-#endif
return False;
}
#ifndef WIN32
@@ -294,10 +292,8 @@
+strlen(ERROR_PREFIX)+strlen(POST_ERROR_MSG1)
+strlen(xkm_output_dir)+strlen(keymap)+44 > PATH_MAX)
{
-#ifdef DEBUG
ErrorF("compiler command for keymap (%s) exceeds max length\n",
names->keymap);
-#endif
return False;
}
#ifndef WIN32
Index: programs/Xserver/xkb/xkbInit.c
diff -u programs/Xserver/xkb/xkbInit.c:1.1.1.2 X11/xc/programs/Xserver/xkb/xkbInit.c:1.3
--- programs/Xserver/xkb/xkbInit.c:1.1.1.2 Sat Mar 7 09:21:55 1998
+++ programs/Xserver/xkb/xkbInit.c Mon Jul 10 15:28:10 2000
@@ -24,7 +24,7 @@
THE USE OR PERFORMANCE OF THIS SOFTWARE.
********************************************************/
-/* $XFree86: xc/programs/Xserver/xkb/xkbInit.c,v 3.12.2.2 1998/02/24 13:20:07 dawes Exp $ */
+/* $XFree86: xc/programs/Xserver/xkb/xkbInit.c,v 3.12.2.3 2000/06/15 21:58:34 dawes Exp $ */
#include <stdio.h>
#include <stdlib.h>
@@ -915,8 +915,13 @@
#endif
else if (strncmp(argv[i], "-xkbmap", 7) == 0) {
if(++i < argc) {
- XkbInitialMap= argv[i];
- return 2;
+ if (strlen(argv[i]) < PATH_MAX) {
+ XkbInitialMap= argv[i];
+ return 2;
+ } else {
+ ErrorF("-xkbmap pathname too long\n");
+ return -1;
+ }
}
else {
return -1;
@@ -924,8 +929,13 @@
}
else if (strncmp(argv[i], "-xkbdb", 7) == 0) {
if(++i < argc) {
- XkbDB= argv[i];
- return 2;
+ if (strlen(argv[i]) < PATH_MAX) {
+ XkbDB= argv[i];
+ return 2;
+ } else {
+ ErrorF("-xkbdb pathname too long\n");
+ return -1;
+ }
}
else {
return -1;
Index: programs/xfs/os/waitfor.c
diff -u programs/xfs/os/waitfor.c:1.1 X11/xc/programs/xfs/os/waitfor.c:1.2
--- programs/xfs/os/waitfor.c:1.1 Fri Sep 5 03:16:07 1997
+++ programs/xfs/os/waitfor.c Mon Jul 10 15:32:38 2000
@@ -1,5 +1,5 @@
/* $XConsortium: waitfor.c /main/15 1996/08/30 14:22:34 kaleb $ */
-/* $XFree86: xc/programs/xfs/os/waitfor.c,v 3.5 1997/01/18 07:02:48 dawes Exp $ */
+/* $XFree86: xc/programs/xfs/os/waitfor.c,v 3.5.2.1 2000/06/15 21:58:35 dawes Exp $ */
/*
* waits for input
*/
@@ -212,7 +212,7 @@
while (clientsReadable.fds_bits[i]) {
curclient = ffs(clientsReadable.fds_bits[i]) - 1;
conn = ConnectionTranslation[curclient + (i << 5)];
- FD_CLR (curclient, &clientsReadable);
+ clientsReadable.fds_bits[i] &= ~(((fd_mask)1L) << curclient);
client = clients[conn];
if (!client)
continue;
--- programs/xauth/process.c.orig Fri Jul 23 06:50:50 1999
+++ programs/xauth/process.c Sat Sep 23 15:31:27 2000
@@ -769,7 +769,7 @@
static int write_auth_file (tmp_nam)
char *tmp_nam;
{
- FILE *fp;
+ FILE *fp = NULL;
AuthList *list;
/*
@@ -778,12 +778,9 @@
strcpy (tmp_nam, xauth_filename);
strcat (tmp_nam, "-n"); /* for new */
(void) unlink (tmp_nam);
- fp = fopen (tmp_nam, "wb"); /* umask is still set to 0077 */
- if (!fp) {
- fprintf (stderr, "%s: unable to open tmp file \"%s\"\n",
- ProgramName, tmp_nam);
- return -1;
- }
+ /* CPhipps 2000/02/12 - fix file unlink/fopen race */
+ fd = open(tmp_nam, O_WRONLY|O_CREAT|O_EXCL, 0600);
+ if (fd != -1) fp = fdopen(fd, "wb");
/*
* Write MIT-MAGIC-COOKIE-1 first, because R4 Xlib knows
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0009231756420.99058-200000>