home *** CD-ROM | disk | FTP | other *** search
- Path: wuarchive!zaphod.mps.ohio-state.edu!usc!apple!bbn!papaya.bbn.com!rsalz
- From: rsalz@bbn.com (Rich Salz)
- Newsgroups: comp.sources.unix
- Subject: v22i010: Patch to indir -- safely execute (set[ug]id) scripts
- Message-ID: <2481@papaya.bbn.com>
- Date: 2 May 90 19:21:34 GMT
- Organization: BBN Systems and Technologies, Cambridge MA
- Lines: 390
- Approved: rsalz@uunet.UU.NET
- X-Checksum-Snefru: b3f85d0f 8d8c5d5a cc236020 fadee82a
-
- 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.
-