rsalz@bbn.com (Rich Salz) (05/03/90)
Submitted-by: Maarten Litmaath <maart@cs.vu.nl> Posting-number: Volume 22, Issue 10 Archive-name: indir.pch Patch-to: volume21/indir There was a small (== not fundamental) problem with indir 2.0. Thanks to J. Greely <jgreely@cis.ohio-state.edu> for pointing it out: >(from setuid.txt) >>Answer: if and only if the file we're reading from is SETUID (setgid) to the >>EFFECTIVE uid (gid) of the process, we know we're executing the original >>script (to be 100% correct: the original link might have been replaced with a >>link to ANOTHER setuid script of the same owner -> merely a waste of time). > >This is not necessarily a waste of time. If there are several scripts >set-id to the same thing, some of which are protected from public >access by directory permissions, indir can't tell which one I'm trying >to execute unless it checks to see if the REAL uid (gid) has >permission to execute the script (at which point it doesn't know that >it's the *right* script, but it's one they could have executed >directly anyway, so no big deal). The main point of indir 2.0 was to make it impossible to execute *arbitrary* scripts instead of the intended setuid (setgid) script, i.e. only setuid (setgid) scripts of the correct owner (group) could be executed; version 2.3 also checks if the script about to get executed really is accessible to the real uid (gid), thereby closing the last hole. Here's an example: % pwd /some/dir % ls -ldg baz foo foo/bar -rwsr-xr-x 1 root wheel 16384 Apr 25 1989 baz drwxr-x--- 12 root wheel 512 Mar 12 23:29 foo -rwsr-x--- 1 root wheel 24576 Mar 5 11:13 foo/bar % id uid=1234(john) gid=56(guest) groups=56(guest) % cd /tmp % ln -s /some/dir/baz baz % /bin/nice -20 ./baz & % rm baz % ln -s /some/dir/foo/bar baz % If the race condition `succeeds', we end up executing `/some/dir/foo/bar' instead of `/some/dir/baz'. Measures: 1) /some/dir/foo/bar isn't executable for `john'; indir 2.3 checks if the file it's reading from *is* executable for the *real* uid (gid), using fstat(2). 2) Even if it was executable, /some/dir/foo wasn't accessible for `john'; indir 2.3 checks if the real uid (gid) can stat(2) its second argument (here `./baz'). To make sure this file really is the file we're reading from, inode and device numbers are compared. 3) If your UNIX version doesn't have symbolic links, you can't link to a file in an inaccessible directory, therefore only point 1 remains. To apply the patches below: in the directory containing the sources of indir 2.0 type `patch < diffs', assuming the cdiffs have been saved in the file `diffs'. Maarten Litmaath @ VU Amsterdam: maart@cs.vu.nl, uunet!mcsun!botter!maart : This is a shar archive. Extract with sh, not csh. : This archive ends with exit, so do not worry about trailing junk. : --------------------------- cut here -------------------------- PATH=/bin:/usr/bin:/usr/ucb echo Extracting 'diffs' sed 's/^X//' > 'diffs' << '+ END-OF-FILE ''diffs' X*** Makefile.B Tue Mar 27 05:14:58 1990 X--- Makefile Tue Mar 27 04:59:43 1990 X*************** X*** 12,19 **** X CC = cc X # if your C library doesn't have strrchr() X # STRRCHR = -Dstrrchr=rindex X X! CFLAGS = -O $(STRRCHR) X X indir: exec_ok $(OBJS) X $(CC) -O -o indir $(OBJS) X--- 12,21 ---- X CC = cc X # if your C library doesn't have strrchr() X # STRRCHR = -Dstrrchr=rindex X+ # if your UNIX version has the union wait (see wait(2)) X+ UNION_WAIT = -DUNION_WAIT X X! CFLAGS = -O $(STRRCHR) $(UNION_WAIT) X X indir: exec_ok $(OBJS) X $(CC) -O -o indir $(OBJS) X*** error.h.B Tue Mar 27 02:10:50 1990 X--- error.h Tue Mar 27 02:11:59 1990 X*************** X*** 40,44 **** X--- 40,45 ---- X ERROR(E_mode, 12, "%s: `%s' is set[ug]id, yet has checking disabled!\n") X ERROR(E_alarm, 13, "%s: `%s' is a fake!\n") X ERROR(E_exec, 14, "%s: cannot execute `%s' in `%s': %s\n") X+ ERROR(E_fork, 15, "%s: cannot fork in `%s': %s\n") X X #endif /* !ERROR_H */ X*** indir.1.B Tue Mar 27 03:07:32 1990 X--- indir.1 Sat Mar 31 00:42:24 1990 X*************** X*** 102,112 **** X 3) before the final \fIexecve\fR(2) X .I indir X checks if the owner and mode of the script are still what they are supposed X! to be (using \fIfstat\fR(2)); if there is a discrepancy, X .I indir X will abort with an error message. X .RE X .PP X .I Indir X will only exec a pathname beginning with a `\fB/\fR', unless the `\-\fIn\fR' X option was specified. In the latter case the user's PATH will be searched X--- 102,121 ---- X 3) before the final \fIexecve\fR(2) X .I indir X checks if the owner and mode of the script are still what they are supposed X! to be (using \fIfstat\fR(2)), and if the user really can access and execute X! it (using \fIstat\fR(2)); \fIinode\fR and \fIdevice\fR numbers are compared X! to make sure all checks refer to the same file. If any discrepancy is found, X .I indir X will abort with an error message. X .RE X .PP X+ Feature: \fIindir\fR always checks if the REAL uid (gid) has access to the X+ setuid (setgid) script, even if the effective id already differed X+ from the real id BEFORE the script was executed. (There isn't even X+ a way to find out the original effective id.) X+ If you want the original effective id to be used, you should set X+ the real id accordingly before executing the script. X+ .PP X .I Indir X will only exec a pathname beginning with a `\fB/\fR', unless the `\-\fIn\fR' X option was specified. In the latter case the user's PATH will be searched X*************** X*** 124,127 **** X .SH BUGS X The maintenance of setuid (setgid) scripts is a bit annoying: if a script is X moved, one must not forget to change the path mentioned in the script. X! Possibly the editing causes a setuid bit to get turned off. X--- 133,136 ---- X .SH BUGS X The maintenance of setuid (setgid) scripts is a bit annoying: if a script is X moved, one must not forget to change the path mentioned in the script. X! Possibly the editing causes a setuid (setgid) bit to get turned off. X*** indir.c.B Tue Mar 27 03:40:36 1990 X--- indir.c Sat Mar 31 00:59:41 1990 X*************** X*** 1,4 **** X! static char sccsid[] = "@(#)indir.c 2.0 89/11/01 Maarten Litmaath"; X X /* X * indir.c X--- 1,4 ---- X! static char sccsid[] = "@(#)indir.c 2.3 90/03/31 Maarten Litmaath"; X X /* X * indir.c X*************** X*** 98,104 **** X * !Uid_check !setuid -> OK: ordinary script X * !Uid_check setuid -> security hole: checking should be enabled X * Uid_check !setuid -> fake X! * Uid_check setuid -> check if st_uid == euid X */ X X if (!Uid_check) { X--- 98,105 ---- X * !Uid_check !setuid -> OK: ordinary script X * !Uid_check setuid -> security hole: checking should be enabled X * Uid_check !setuid -> fake X! * Uid_check setuid -> check if st_uid == euid and if user has X! * access permissions X */ X X if (!Uid_check) { X*************** X*** 112,121 **** X } else { X /* X * Check if the file we're reading from is setuid and owned X! * by geteuid(). If this test fails, the file is a fake, X! * else it MUST be ok! X */ X! if (!(st.st_mode & S_ISUID) || st.st_uid != geteuid()) X error(E_alarm, Prog, File); X } X X--- 113,125 ---- X } else { X /* X * Check if the file we're reading from is setuid and owned X! * by geteuid(). If this test fails, the file is a fake. X! * Then check if the real uid can execute the file at all: X! * he could have used a link()/unlink() scheme to get us to X! * execute an inaccessible script. X */ X! if (!(st.st_mode & S_ISUID) || st.st_uid != geteuid() X! || chkperm(&st, File) < 0) X error(E_alarm, Prog, File); X } X X*************** X*** 125,137 **** X if (st.st_mode & S_ISGID) X error(E_mode, Prog, File); X } else { X! if (!(st.st_mode & S_ISGID) || st.st_gid != getegid()) X error(E_alarm, Prog, File); X } X X /* X * If we're executing a set[ug]id file, replace the complete X! * environment by a save default, else permit the PATH to be X * searched too. X */ X if (st.st_mode & (S_ISUID | S_ISGID)) X--- 129,142 ---- X if (st.st_mode & S_ISGID) X error(E_mode, Prog, File); X } else { X! if (!(st.st_mode & S_ISGID) || st.st_gid != getegid() X! || chkperm(&st, File) < 0) X error(E_alarm, Prog, File); X } X X /* X * If we're executing a set[ug]id file, replace the complete X! * environment by a safe default, else permit the PATH to be X * searched too. X */ X if (st.st_mode & (S_ISUID | S_ISGID)) X*************** X*** 252,257 **** X--- 257,335 ---- X while ((c = *p++) != ' ' && c != '\t' && c != '\n') X ; X return --p; X+ } X+ X+ X+ #define X_USR S_IXUSR /* 0100 */ X+ #define X_GRP S_IXGRP /* 0010 */ X+ #define X_OTH S_IXOTH /* 0001 */ X+ X+ X+ static int chkperm(st, f) X+ struct stat *st; X+ char *f; X+ { X+ struct stat stbuf; X+ int xmask, pid; X+ uid_t uid; X+ gid_t gid; X+ static int status = -1, checked = 0; X+ #ifdef UNION_WAIT X+ union wait w; X+ #define ok(w) (w.w_status == 0) X+ #else X+ int w; X+ #define ok(w) (w == 0) X+ #endif /* UNION_WAIT */ X+ X+ if (checked) X+ return status; X+ checked = 1; X+ X+ /* X+ * If `Uid_check' (`Gid_check') has been set, we're executing a X+ * setuid (setgid) script, so use the real uid (gid) in the access X+ * check; else use the effective uid (gid), just like the kernel does. X+ * Feature: we always check if the REAL uid (gid) has access to the X+ * setuid (setgid) script, even if the effective id already differed X+ * from the real id BEFORE the script was executed. (There isn't even X+ * a way to find out the original effective id.) X+ * If you want the original effective id to be used, you should set X+ * the real id accordingly before executing the script. X+ */ X+ uid = Uid_check ? Uid : geteuid(); X+ gid = Gid_check ? getgid() : getegid(); X+ X+ xmask = (Uid == 0) ? (X_USR | X_GRP | X_OTH) : X+ st->st_uid == uid ? X_USR : X+ st->st_gid == gid ? X_GRP : X+ X_OTH; X+ /* X+ * Can the invoker really execute the file we're reading from? X+ */ X+ if (!(st->st_mode & xmask)) X+ return -1; X+ X+ switch (pid = fork()) { X+ case -1: X+ error(E_fork, Prog, File, geterr()); X+ case 0: X+ if (Uid_check) X+ (void) setuid(uid); /* reset uid */ X+ if (Gid_check) X+ (void) setgid(gid); /* reset gid */ X+ /* X+ * Now check if the `real' uid (gid) can access the file X+ * we're reading from, i.e. if the leading directories are X+ * searchable. Compare `st_ino' and `st_dev' to make sure X+ * we're talking about the same file. X+ */ X+ exit(stat(f, &stbuf) < 0 || stbuf.st_ino != st->st_ino X+ || stbuf.st_dev != st->st_dev); X+ } X+ while (wait(&w) != pid) X+ ; X+ return ok(w) ? status = 0 : -1; X } X X X*** indir.h.B Tue Mar 27 04:47:03 1990 X--- indir.h Tue Mar 27 04:47:34 1990 X*************** X*** 1,6 **** X--- 1,9 ---- X #include <sys/param.h> X #include <sys/stat.h> X #include <stdio.h> X+ #ifdef UNION_WAIT X+ #include <sys/wait.h> X+ #endif /* UNION_WAIT */ X X #define COMMENT '#' X #define MAGIC '?' X*** setuid.txt.B Tue Mar 27 02:45:31 1990 X--- setuid.txt Sat Mar 31 00:26:58 1990 X*************** X*** 103,114 **** X the link to the script might have been quickly replaced with a link to another X script, i.e. how can we trust this `#?' line? X Answer: if and only if the file we're reading from is SETUID (setgid) to the X! EFFECTIVE uid (gid) of the process, we know we're executing the original X! script (to be 100% correct: the original link might have been replaced with a X! link to ANOTHER setuid script of the same owner -> merely a waste of time). X! To reliably check the condition stated above, we use fstat(2) on the file X! descriptor we're reading from. Can you figure out why stat(2) would be X! insecure? X To deal with IFS, PATH and other environment problems, indir(1) resets the X environment to a simple default: X X--- 103,125 ---- X the link to the script might have been quickly replaced with a link to another X script, i.e. how can we trust this `#?' line? X Answer: if and only if the file we're reading from is SETUID (setgid) to the X! EFFECTIVE uid (gid) of the process, AND it's accessible and executable for X! the REAL uid (gid), we know we're executing the original script (to be 100% X! correct: the original link might have been replaced with a link to ANOTHER X! accessible and executable setuid (setgid) script of the same owner (group) X! -> merely a waste of time). X! To check the condition stated above reliably, we use fstat(2) on the file X! descriptor we're reading from, and stat(2) on the associated file name. X! We compare inode and device numbers to make sure we're talking about the X! same file. Can you figure out why using stat(2) alone would be insecure? X! X! Feature: we always check if the REAL uid (gid) has access to the setuid X! (setgid) script, even if the effective id already differed from the real id X! BEFORE the script was executed. (There isn't even a way to find out the X! original effective id.) X! If you want the original effective id to be used, you should set the real id X! accordingly before executing the script. X! X To deal with IFS, PATH and other environment problems, indir(1) resets the X environment to a simple default: X + END-OF-FILE diffs chmod 'u=rw,g=r,o=r' 'diffs' set `wc -c 'diffs'` count=$1 case $count in 10201) :;; *) echo 'Bad character count in ''diffs' >&2 echo 'Count should be 10201' >&2 esac exit 0 -- Please send comp.sources.unix-related mail to rsalz@uunet.uu.net. Use a domain-based address or give alternate paths, or you may lose out.