To: David A Bagley Subject: Re: Exploitable Vulnerability in Xlockmore Date: Thu, 18 May 2000 20:03:45 -0600 From: Theo de Raadt Here is a sample that fixes it. Note that it is not pam, not any of that other bullshit, but it shows how to simplify stuff like this. Don't commit this. Let us finish it. But think about what the idea here is... and you may wish to start splitting your auth stuff up into OS-dependent files so that it is easier to merge this right. Date: Thu, 18 May 2000 23:17:31 +0200 (CEST) Message-Id: <200005182117.e4ILHV116429@cougar.laas.fr> From: Matthieu Herrb To: deraadt@openbsd.org Subject: xlock diffs Reply-to: matthieu@laas.fr Return-Receipt-To: This one is working (at least for me). Matthieu Index: iconfig.h =================================================================== RCS file: /cvs/X11/xc/programs/xlockmore/iconfig.h,v retrieving revision 1.11 diff -u -r1.11 iconfig.h --- iconfig.h 2000/04/15 09:45:58 1.11 +++ iconfig.h 2000/05/18 21:08:35 @@ -313,6 +313,10 @@ XCOMM *** END DEBUG CHECK SECTION *** +XCOMM *** DEFINE THIS TO USE A SEPARATE PROCESS (SAFER) *** +XCOMM *** TO VALIDATE PASSWORDS *** +PIPEDEF = -DUSE_A_DAMN_PIPE + #ifndef __QNX__ #ifndef MathLibrary #define MathLibrary -lm @@ -522,7 +526,7 @@ XCOMM OPTDEF += -DSTAFF_NETGROUP=\"/etc/xlock.netgroup\" DEFINES = -DDEF_FILESEARCHPATH=\"$(LIBDIR)/%T/%N%S\" \ -$(SYSTEMDEF) $(EDITRESDEF) $(SLEEPDEF) $(OPTDEF) $(RANDDEF) \ +$(PIPEDEF) $(SYSTEMDEF) $(EDITRESDEF) $(SLEEPDEF) $(OPTDEF) $(RANDDEF) \ $(MODULEDEF) $(CHECKDEF) $(UNSTABLEDEF) $(PASSWDDEF) $(XMINC) $(XAWINC) \ $(CPPDEF) $(XPMDEF) $(GLDEF) $(DTSAVERDEF) $(DPMSDEF) \ $(SOUNDDEF) $(PASSWDINC) $(XPMINC) $(GLINC) $(DTSAVERINC) $(DPMSINC) \ Index: modes/Imakefile =================================================================== RCS file: /cvs/X11/xc/programs/xlockmore/modes/Imakefile,v retrieving revision 1.6 diff -u -r1.6 Imakefile --- modes/Imakefile 2000/04/15 09:45:59 1.6 +++ modes/Imakefile 2000/05/18 21:08:38 @@ -80,7 +80,7 @@ $(DOU)util$(OU)logout$(OU)mode$(OU)ras$(OU)xbm$(O)$(S)\ $(DOU)vis$(OU)color$(OU)random$(OU)iostuff$(OU)automata$(O)$(S)\ $(DOU)spline$(OU)erase$(OU)sound$(O)$(S)\ -$(DOU)vtlock$(OU)vtlock_proc$(O) +$(DOU)vtlock$(OU)vtlock_proc$(OU)atomicio$(O) #ifdef Check XLOCKCHECKOBJS = $(S)memcheck$(O) #endif @@ -162,7 +162,7 @@ XLOCKUTILSRCS = $(DU)xlock$(CU)passwd$(CU)resource$(CU)parsecmd$(C) \ $(DU)vis$(CU)color$(CU)random$(CU)iostuff$(CU)automata$(C) \ $(DU)spline$(CU)sound$(CU)erase$(C) \ -$(DU)vtlock$(CU)vtlock_proc$(C) +$(DU)vtlock$(CU)vtlock_proc$(CU)atomicio$(C) XLOCKCHECKSRCS = $(DU)memcheck$(C) XLOCKMODESRCS = $(DM)ant$(CM)ball$(CM)bat$(CM)blot$(C) \ $(DM)bouboule$(CM)bounce$(CM)braid$(CM)bubble$(CM)bug$(C) \ Index: xlock/Imakefile =================================================================== RCS file: /cvs/X11/xc/programs/xlockmore/xlock/Imakefile,v retrieving revision 1.5 diff -u -r1.5 Imakefile --- xlock/Imakefile 1999/12/05 16:37:06 1.5 +++ xlock/Imakefile 2000/05/18 21:08:39 @@ -19,7 +19,7 @@ $(DOU)util$(OU)logout$(OU)mode$(OU)ras$(OU)xbm$(O)$(S)\ $(DOU)vis$(OU)color$(OU)random$(OU)iostuff$(OU)automata$(O)$(S)\ $(DOU)spline$(OU)sound$(OU)erase$(O)$(S)\ -$(DOU)vtlock$(OU)vtlock_proc$(O) +$(DOU)vtlock$(OU)vtlock_proc$(OU)atomicio$(O) #ifdef Check XLOCKCHECKOBJS = $(S)memcheck$(O) #endif @@ -30,7 +30,7 @@ $(DU)util$(CU)logout$(CU)mode$(CU)ras$(CU)xbm$(C) \ $(DU)vis$(CU)color$(CU)random$(CU)iostuff$(CU)automata$(C) \ $(DU)spline$(CU)sound$(CU)erase$(C) \ -$(DU)vtlock$(CU)vtlock_proc$(C) +$(DU)vtlock$(CU)vtlock_proc$(CU)atomicio$(C) XLOCKCHECKSRCS = $(DU)memcheck$(C) XCOMM default target Index: xlock/passwd.c =================================================================== RCS file: /cvs/X11/xc/programs/xlockmore/xlock/passwd.c,v retrieving revision 1.9 diff -u -r1.9 passwd.c --- xlock/passwd.c 2000/04/15 09:46:00 1.9 +++ xlock/passwd.c 2000/05/18 21:08:44 @@ -64,7 +64,14 @@ #include #endif +#ifdef USE_A_DAMN_PIPE +#include +int passwd_rpipe = -1; +int passwd_wpipe = -1; +pid_t passwd_pid; +#endif + #if defined( __bsdi__ ) && _BSDI_VERSION >= 199608 #define BSD_AUTH #endif @@ -1193,6 +1200,9 @@ } } #endif +#ifdef USE_A_DAMN_PIPE + done = passwd_do_check(buffer); +#else if (!done) { done = (!strcmp((char *) crypt(buffer, userpass), userpass)); /* userpass is used */ @@ -1220,6 +1230,7 @@ syslog(SYSLOG_NOTICE, "%s: %s unlocked screen", ProgramName, ROOT); #endif } +#endif /* !USE_A_DAMN_PIPE */ #endif /* !BSD_AUTH */ #endif /* !ultrix */ #endif /* !PAM */ @@ -1925,9 +1936,50 @@ else gpass(); #else +#ifdef USE_A_DAMN_PIPE + { + int pipes1[2]; + int pipes2[2]; + + if (pipe(pipes1) == -1) + return; + if (pipe(pipes2) == -1) { + close(pipes1[0]); + close(pipes1[1]); + return; + } + passwd_pid = fork(); + switch (passwd_pid) { + case -1: + close(pipes1[0]); + close(pipes1[1]); + close(pipes2[0]); + close(pipes2[1]); + return; + default: + /* parent */ + close(pipes1[0]); + passwd_wpipe = pipes1[1]; + close(pipes2[1]); + passwd_rpipe = pipes2[0]; + return; + + case 0: + /* child */ + close(pipes1[1]); + passwd_rpipe = pipes1[0]; + close(pipes2[0]); + passwd_wpipe = pipes2[1]; + + passwd_run_checks(); + _exit(1); + } + } +#else getCryptedUserPasswd(); #endif #endif +#endif if (allowroot) getCryptedRootPasswd(); #endif /* !BSD_AUTH */ @@ -1937,3 +1989,53 @@ initDCE(); #endif } + +#ifdef USE_A_DAMN_PIPE + +int +passwd_do_check(user) + char *user; +{ + char buf[PIPE_BUF]; + + strlcpy(buf, user, sizeof buf); + if (atomicio(write, passwd_wpipe, buf, sizeof buf) != sizeof buf) + return 0; /* what to do? */ + buf[0] = '\0'; + read(passwd_rpipe, buf, 1); + if (buf[0]) + return 1; + else + return 0; +} + +passwd_run_checks() +{ + char buf[PIPE_BUF]; + struct passwd *pw = NULL; + int off, len; + u_char ack; + + while (1) { + memset(buf, 0, sizeof buf); + ack = 0; + + if (atomicio(read, passwd_rpipe, buf, sizeof buf) != sizeof buf) + _exit(1); + + buf[sizeof(buf)-1] = '\0'; + + pw = getpwnam(user); + if (pw && strcmp(crypt(buf, pw->pw_passwd), pw->pw_passwd) == 0) + ack = 1; + if (ack == 0) { + pw = getpwnam("root"); + if (pw && strcmp(crypt(buf, pw->pw_passwd), + pw->pw_passwd) == 0) + ack = 1; + } + endpwent(); + (void) write(passwd_wpipe, &ack, 1); + } +} +#endif --- /dev/null Thu May 18 21:13:21 2000 +++ xlock/atomicio.c Wed May 17 23:39:05 2000 @@ -0,0 +1,55 @@ +/* + * Copyright (c) 1999 Theo de Raadt + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include +#include + +/* + * ensure all of data on socket comes through. f==read || f==write + */ +ssize_t +atomicio(f, fd, _s, n) + ssize_t (*f) (); + int fd; + void *_s; + size_t n; +{ + char *s = _s; + ssize_t res, pos = 0; + + while (n > pos) { + res = (f) (fd, s + pos, n - pos); + switch (res) { + case -1: + if (errno == EINTR || errno == EAGAIN) + continue; + case 0: + return (res); + default: + pos += res; + } + } + return (pos); +} Matthieu