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: X11/xc/lib/ICE/ICElibint.h diff -u X11/xc/lib/ICE/ICElibint.h:1.1 X11/xc/lib/ICE/ICElibint.h:1.2 --- X11/xc/lib/ICE/ICElibint.h:1.1 Fri Sep 5 02:58:32 1997 +++ X11/xc/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: X11/xc/lib/ICE/process.c diff -u X11/xc/lib/ICE/process.c:1.1 X11/xc/lib/ICE/process.c:1.2 --- X11/xc/lib/ICE/process.c:1.1 Fri Sep 5 02:58:32 1997 +++ X11/xc/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: X11/xc/lib/X11/GetProp.c diff -u X11/xc/lib/X11/GetProp.c:1.1 X11/xc/lib/X11/GetProp.c:1.2 --- X11/xc/lib/X11/GetProp.c:1.1 Fri Sep 5 02:58:44 1997 +++ X11/xc/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: X11/xc/lib/X11/OpenDis.c diff -u X11/xc/lib/X11/OpenDis.c:1.1 X11/xc/lib/X11/OpenDis.c:1.2 --- X11/xc/lib/X11/OpenDis.c:1.1 Fri Sep 5 02:58:48 1997 +++ X11/xc/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: X11/xc/lib/X11/XlibInt.c diff -u X11/xc/lib/X11/XlibInt.c:1.3 X11/xc/lib/X11/XlibInt.c:1.4 --- X11/xc/lib/X11/XlibInt.c:1.3 Tue Aug 24 12:11:19 1999 +++ X11/xc/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 #include @@ -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: X11/xc/programs/Xserver/os/secauth.c diff -u X11/xc/programs/Xserver/os/secauth.c:1.1 X11/xc/programs/Xserver/os/secauth.c:1.3 --- X11/xc/programs/Xserver/os/secauth.c:1.1 Fri Sep 5 03:15:14 1997 +++ X11/xc/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: X11/xc/programs/Xserver/os/xdmcp.c diff -u X11/xc/programs/Xserver/os/xdmcp.c:1.1.1.2 X11/xc/programs/Xserver/os/xdmcp.c:1.2 --- X11/xc/programs/Xserver/os/xdmcp.c:1.1.1.2 Fri Jan 8 10:56:48 1999 +++ X11/xc/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: X11/xc/programs/Xserver/xkb/ddxLoad.c diff -u X11/xc/programs/Xserver/xkb/ddxLoad.c:1.1.1.3 X11/xc/programs/Xserver/xkb/ddxLoad.c:1.2 --- X11/xc/programs/Xserver/xkb/ddxLoad.c:1.1.1.3 Sat Nov 28 01:49:13 1998 +++ X11/xc/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 #include @@ -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: X11/xc/programs/Xserver/xkb/xkbInit.c diff -u X11/xc/programs/Xserver/xkb/xkbInit.c:1.1.1.2 X11/xc/programs/Xserver/xkb/xkbInit.c:1.3 --- X11/xc/programs/Xserver/xkb/xkbInit.c:1.1.1.2 Sat Mar 7 09:21:55 1998 +++ X11/xc/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 #include @@ -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: X11/xc/programs/xauth/process.c diff -u X11/xc/programs/xauth/process.c:1.2 X11/xc/programs/xauth/process.c:1.3 --- X11/xc/programs/xauth/process.c:1.2 Tue Feb 15 00:15:35 2000 +++ X11/xc/programs/xauth/process.c Mon Jul 10 15:30:21 2000 @@ -769,7 +769,7 @@ static int write_auth_file (tmp_nam) char *tmp_nam; { - FILE *fp; + FILE *fp = NULL; int fd; AuthList *list; @@ -779,7 +779,6 @@ 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 */ /* 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"); Index: X11/xc/programs/xfs/os/waitfor.c diff -u X11/xc/programs/xfs/os/waitfor.c:1.1 X11/xc/programs/xfs/os/waitfor.c:1.2 --- X11/xc/programs/xfs/os/waitfor.c:1.1 Fri Sep 5 03:16:07 1997 +++ X11/xc/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;