diff -urN xc.orig/config/cf/Server.tmpl xc/config/cf/Server.tmpl --- xc.orig/config/cf/Server.tmpl Mon Dec 30 15:52:30 2002 +++ xc/config/cf/Server.tmpl Mon Dec 30 17:31:51 2002 @@ -25,8 +25,14 @@ #ifndef DoThreadedServer #define DoThreadedServer NO #endif +#ifndef XserverNeedsSetUID +#define XserverNeedsSetUID NO +#endif +#ifndef UseXserverWrapper +#define UseXserverWrapper XserverNeedsSetUID +#endif #ifndef InstallServerSetUID -#define InstallServerSetUID NO +#define InstallServerSetUID (XserverNeedsSetUID && !UseXserverWrapper) #endif #ifdef CrossCompileDir diff -urN xc.orig/config/cf/xf86site.def xc/config/cf/xf86site.def --- xc.orig/config/cf/xf86site.def Mon Dec 30 15:52:31 2002 +++ xc/config/cf/xf86site.def Mon Dec 30 17:37:06 2002 @@ -69,14 +69,15 @@ */ /* - * If you only run the X server under xdm the X servers don't need to be - * installed SetUID, and you may comment out the lines below. If you run - * the servers by hand (with xinit or startx), then they do need to be - * installed SetUID on most platforms. + * The X servers need to run as root on most OSs. We're now using a + * wrapper in that case, but we still need to make it known that the + * servers need SetUID. When only using xdm, this (and the wrapper) + * are not required. Disabling this automatically disables use of the + * wrapper. * - * Consult your system administrator before making the X server setuid. + * If you're only starting the Xservers with xdm set this to NO * -#define InstallXserverSetUID NO +#define XserverNeedsSetUID NO */ diff -urN xc.orig/config/cf/xfree86.cf xc/config/cf/xfree86.cf --- xc.orig/config/cf/xfree86.cf Mon Dec 30 15:52:31 2002 +++ xc/config/cf/xfree86.cf Mon Dec 30 17:31:51 2002 @@ -1104,12 +1104,15 @@ #endif /* - * The default is to install the X servers setuid-root on most OSs. - * It the servers are only started by xdm, they should not be setuid-root. + * The X servers need to run as root on most OSs. We're now using a + * wrapper in that case, but we still need to make it known that the + * servers need SetUID. When only using xdm, this (and the wrapper) + * are not required. Disabling this automatically disables use of the + * wrapper. */ #if !defined(i386MachArchitecture) && !defined(OS2Architecture) -# ifndef InstallXserverSetUID -# define InstallXserverSetUID YES +# ifndef XserverNeedsSetUID +# define XserverNeedsSetUID YES # endif #endif diff -urN xc.orig/programs/Xserver/Imakefile xc/programs/Xserver/Imakefile --- xc.orig/programs/Xserver/Imakefile Mon Dec 30 15:53:22 2002 +++ xc/programs/Xserver/Imakefile Mon Dec 30 17:31:51 2002 @@ -4,11 +4,6 @@ */ XCOMM $XFree86: xc/programs/Xserver/Imakefile,v 3.275 2002/12/21 00:19:11 torrey Exp $ -#ifndef InstallXserverSetUID -#define InstallXserverSetUID NO -#endif -#define InstallServerSetUID InstallXserverSetUID - #include #ifdef XFree86Version @@ -1146,6 +1141,11 @@ #endif /* XnestServer */ +#if UseXserverWrapper +SetUIDProgramTarget(Xwrapper,os/wrapper.o,NullParameter,$(PAMLIBS),NullParameter) +InstallProgramWithFlags(Xwrapper,$(BINDIR),$(INSTUIDFLAGS)) +#endif + #if defined(XnonServer) && XnonServer XCOMM XCOMM non server, just compile sources for build test diff -urN xc.orig/programs/Xserver/hw/xfree86/os-support/linux/lnx_init.c xc/programs/Xserver/hw/xfree86/os-support/linux/lnx_init.c --- xc.orig/programs/Xserver/hw/xfree86/os-support/linux/lnx_init.c Mon Dec 30 15:53:52 2002 +++ xc/programs/Xserver/hw/xfree86/os-support/linux/lnx_init.c Mon Dec 30 17:31:51 2002 @@ -66,7 +66,10 @@ /* check if we're run with euid==0 */ if (geteuid() != 0) { - FatalError("xf86OpenConsole: Server must be suid root\n"); + FatalError("xf86OpenConsole: Server must be running with root " + "permissions\n" + "You should be using Xwrapper to start the server or xdm.\n" + "We strongly advise against making the server SUID root!\n"); } /* diff -urN xc.orig/programs/Xserver/os/Imakefile xc/programs/Xserver/os/Imakefile --- xc.orig/programs/Xserver/os/Imakefile Mon Dec 30 15:53:58 2002 +++ xc/programs/Xserver/os/Imakefile Mon Dec 30 17:31:52 2002 @@ -120,6 +120,7 @@ INCLUDES = -I. -I../include -I$(XINCLUDESRC) -I$(EXTINCSRC) \ -I$(SERVERSRC)/Xext -I$(FONTINCSRC) -I$(SERVERSRC)/render \ -I$(TOP)/lib/Xau -I../lbx Krb5Includes + EXTRA_DEFINES = -DUSE_PAM DEPEND_DEFINES = $(DBM_DEFINES) $(XDMCP_DEFINES) $(EXT_DEFINES) \ $(TRANS_INCLUDES) $(CONNECTION_FLAGS) DependDefines LINTLIBS = ../dix/llib-ldix.ln @@ -166,6 +167,14 @@ SpecialCObjectRule(oscolor,$(ICONFIGFILES),$(DBM_DEFINES)) #endif +#if UseXserverWrapper +AllTarget(wrapper.o) + + WRAPPER_DEFINES = -DXSERVER_PATH=\"/etc/X11/X\" + +SpecialCObjectRule(wrapper,NullParameter,$(WRAPPER_DEFINES)) +#endif + #if HasKrb5 LinkSourceFile(k5encode.c,$(XAUTHSRC)) #endif diff -urN xc.orig/programs/Xserver/os/wrapper.c xc/programs/Xserver/os/wrapper.c --- xc.orig/programs/Xserver/os/wrapper.c Thu Jan 1 01:00:00 1970 +++ xc/programs/Xserver/os/wrapper.c Mon Dec 30 17:31:52 2002 @@ -0,0 +1,304 @@ +/* + * X server wrapper. + * + * This wrapper makes some sanity checks on the command line arguments + * and environment variables when run with euid == 0 && euid != uid. + * If the checks fail, the wrapper exits with a message. + * If they succeed, it exec's the Xserver. + */ + +/* + * Copyright (c) 1998 by The XFree86 Project, Inc. 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 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 XFREE86 PROJECT 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. + * + * Except as contained in this notice, the name of the XFree86 Project + * shall not be used in advertising or otherwise to promote the sale, + * use or other dealings in this Software without prior written + * authorization from the XFree86 Project. + */ + +/* $XFree86: xc/programs/Xserver/os/wrapper.c,v 1.1.2.5 1998/02/27 15:28:59 dawes Exp $ */ + +/* This is normally set in the Imakefile */ +#ifndef XSERVER_PATH +#define XSERVER_PATH "/etc/X11/X" +#endif + +#include +#include +#include +#include +#include +#include +#ifdef USE_PAM +#include +#include +#include +#endif /* USE_PAM */ + +/* Neither of these should be required for XFree86 3.3.2 */ +#ifndef REJECT_CONFIG +#define REJECT_CONFIG 0 +#endif +#ifndef REJECT_XKBDIR +#define REJECT_XKBDIR 0 +#endif + +/* Consider LD* variables insecure ? */ +#ifndef REMOVE_ENV_LD +#define REMOVE_ENV_LD 1 +#endif + +/* Remove long environment variables? */ +#ifndef REMOVE_LONG_ENV +#define REMOVE_LONG_ENV 1 +#endif + +/* Check args and env only if running setuid (euid == 0 && euid != uid) ? */ +#ifndef CHECK_EUID +#define CHECK_EUID 1 +#endif + +/* + * Maybe the locale can be faked to make isprint(3) report that everything + * is printable? Avoid it by default. + */ +#ifndef USE_ISPRINT +#define USE_ISPRINT 0 +#endif + +#define MAX_ARG_LENGTH 128 +#define MAX_ENV_LENGTH 256 +#define MAX_ENV_PATH_LENGTH 2048 + +#if USE_ISPRINT +#include +#define checkPrintable(c) isprint(c) +#else +#define checkPrintable(c) (((c) & 0x7f) >= 0x20 && ((c) & 0x7f) != 0x7f) +#endif + +enum BadCode { + NotBad = 0, + UnsafeArg, + ArgTooLong, + UnprintableArg, + EnvTooLong, + InternalError, +#ifdef USE_PAM + PamFailed, + PamAuthFailed, +#endif /* USE_PAM */ +}; + +#define ARGMSG \ + "\nIf the arguments used are valid, and have been rejected incorrectly\n" \ + "please send details of the arguments and why they are valid to\n" \ + "XFree86@XFree86.org. In the meantime, you can start the Xserver as\n" \ + "the \"super user\" (root).\n" + +#define ENVMSG \ + "\nIf the environment is valid, and have been rejected incorrectly\n" \ + "please send details of the environment and why it is valid to\n" \ + "XFree86@XFree86.org. In the meantime, you can start the Xserver as\n" \ + "the \"super user\" (root).\n" + +#ifdef USE_PAM +static struct pam_conv conv = { + misc_conv, + NULL +}; +#endif /* USE_PAM */ + + +int +main(int argc, char **argv, char **envp) +{ + enum BadCode bad = NotBad; + int i, j; + char *a, *e; +#ifdef USE_PAM + pam_handle_t *pamh = NULL; + struct passwd *pw; + int retval; + + pw = getpwuid(getuid()); + if (pw == NULL) { + bad = InternalError; + } + + if (!bad) { + retval = pam_start("xserver", pw->pw_name, &conv, &pamh); + if (retval != PAM_SUCCESS) + bad = PamFailed; + } + + if (!bad) { + retval = pam_authenticate(pamh, 0); + if (retval != PAM_SUCCESS) { + pam_end(pamh, retval); + bad = PamAuthFailed; + } + } + + if (!bad) { + retval = pam_acct_mgmt(pamh, 0); + if (retval != PAM_SUCCESS) { + pam_end(pamh, retval); + bad = PamAuthFailed; + } + } + + /* this is not a session, so do not do session management */ + + if (!bad) pam_end(pamh, PAM_SUCCESS); +#endif /* USE_PAM */ + +#if CHECK_EUID + if (!bad && geteuid() == 0 && getuid() != geteuid()) { +#else + if (!bad) { +#endif + /* Check each argv[] */ + for (i = 1; i < argc; i++) { + + /* Check for known bad arguments */ +#if REJECT_CONFIG + if (strcmp(argv[i], "-config") == 0) { + bad = UnsafeArg; + break; + } +#endif +#if REJECT_XKBDIR + if (strcmp(argv[i], "-xkbdir") == 0) { + bad = UnsafeArg; + break; + } +#endif + if (strlen(argv[i]) > MAX_ARG_LENGTH) { + bad = ArgTooLong; + break; + } + a = argv[i]; + while (*a) { + if (checkPrintable(*a) == 0) { + bad = UnprintableArg; + break; + } + a++; + } + if (bad) + break; + } + /* Check each envp[] */ + if (!bad) + for (i = 0; envp[i]; i++) { + + /* Check for bad environment variables and values */ +#if REMOVE_ENV_LD + while (envp[i] && (strncmp(envp[i], "LD", 2) == 0)) { + for (j = i; envp[j]; j++) { + envp[j] = envp[j+1]; + } + } +#endif + if (envp[i] && (strlen(envp[i]) > MAX_ENV_LENGTH)) { +#if REMOVE_LONG_ENV + for (j = i; envp[j]; j++) { + envp[j] = envp[j+1]; + } + i--; +#else + char *eq; + int len; + + eq = strchr(envp[i], '='); + if (!eq) + continue; + len = eq - envp[i]; + e = malloc(len + 1); + if (!e) { + bad = InternalError; + break; + } + strncpy(e, envp[i], len); + e[len] = 0; + if (len >= 4 && + (strcmp(e + len - 4, "PATH") == 0 || + strcmp(e, "TERMCAP") == 0)) { + if (strlen(envp[i]) > MAX_ENV_PATH_LENGTH) { + bad = EnvTooLong; + break; + } else { + free(e); + } + } else { + bad = EnvTooLong; + break; + } +#endif + } + } + } + switch (bad) { + case NotBad: + execve(XSERVER_PATH, argv, envp); + fprintf(stderr, "execve failed for %s (errno %d)\n", XSERVER_PATH, + errno); + break; + case UnsafeArg: + fprintf(stderr, "Command line argument number %d is unsafe\n", i); + fprintf(stderr, ARGMSG); + break; + case ArgTooLong: + fprintf(stderr, "Command line argument number %d is too long\n", i); + fprintf(stderr, ARGMSG); + break; + case UnprintableArg: + fprintf(stderr, "Command line argument number %d contains unprintable" + " characters\n", i); + fprintf(stderr, ARGMSG); + break; + case EnvTooLong: + fprintf(stderr, "Environment variable `%s' is too long\n", e); + fprintf(stderr, ENVMSG); + break; + case InternalError: + fprintf(stderr, "Internal Error\n"); + break; +#ifdef USE_PAM + case PamFailed: + fprintf(stderr, "Authentication System Failure, " + "missing or mangled PAM configuration file or module?\n"); + break; + case PamAuthFailed: + fprintf(stderr, "PAM authentication failed\n"); + break; +#endif + default: + fprintf(stderr, "Unknown error\n"); + fprintf(stderr, ARGMSG); + fprintf(stderr, ENVMSG); + break; + } + exit(1); +} + diff -urN xc.orig/programs/xinit/startx.cpp xc/programs/xinit/startx.cpp --- xc.orig/programs/xinit/startx.cpp Mon Dec 30 15:54:10 2002 +++ xc/programs/xinit/startx.cpp Mon Dec 30 17:31:52 2002 @@ -53,7 +53,7 @@ sysclientrc=XINITDIR/xinitrc sysserverrc=XINITDIR/xserverrc defaultclient=BINDIR/xterm -defaultserver=BINDIR/X +defaultserver=BINDIR/Xwrapper defaultclientargs="" defaultserverargs="" clientargs="" diff -urN xc.orig/programs/xinit/xinit.c xc/programs/xinit/xinit.c --- xc.orig/programs/xinit/xinit.c Mon Dec 30 15:54:10 2002 +++ xc/programs/xinit/xinit.c Mon Dec 30 17:31:52 2002 @@ -146,6 +146,7 @@ #define OK_EXIT 0 #define ERR_EXIT 1 +char *default_wrapper = BINDIR "/Xwrapper"; char *default_server = "X"; char *default_display = ":0"; /* choose most efficient */ char *default_client[] = {"xterm", "-geometry", "+1+1", "-n", "login", NULL}; @@ -332,7 +333,10 @@ if (argc == 0 || #ifndef __UNIXOS2__ (**argv != '/' && **argv != '.')) { - *sptr++ = default_server; + if (access(default_wrapper, X_OK) == 0) + *sptr++ = default_wrapper; + else + *sptr++ = default_server; #else (**argv != '/' && **argv != '\\' && **argv != '.' && !(isalpha(**argv) && (*argv)[1]==':'))) {