maart@nat.vu.nl (Maarten Litmaath) (02/20/91)
I've sent these diffs to comp.sources.unix, but as that group has been rather quiet lately, posting them to alt.sources as well seems justified... Included is a second minor patch to indir (from version 2.3 to 2.4). Version 2.3 did not support multiple groups (see getgroups(2)). The new version has this corrected. Thanks to John M. Sellens <jmsellens@watdragon.uwaterloo.ca> (and Andy at the same site) for pointing it out. He also mentioned that the Makefile and the test script assumed the installer had `.' in his PATH. This has been fixed too. Finally there are a few cosmetic changes to a few files. To apply the patches below, in the directory containing the sources of indir 2.3 unpack the shell archive and type: patch < indir2.4diffs Then check the file `README' and the Makefile. Type `make' to create the new version of indir in the current directory. Enjoy. BTW, please note that my email address has changed. Maarten Litmaath <maart@nat.vu.nl> ^^^ : 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 'indir2.4diffs' sed 's/^X//' > 'indir2.4diffs' << '+ END-OF-FILE ''indir2.4diffs' X*** old/Makefile Sat Feb 2 07:19:48 1991 X--- Makefile Sat Feb 2 07:23:38 1991 X*************** X*** 5,11 **** X # a few defines for testing purposes; if you test in the current X # directory the paths needn't be absolute X # the path of indir will be used in #! lines -> max. 32 characters for X! # #! + name + mandatory option X PATH_OF_INDIR = ./indir X TEST_DIRECTORY = . X X--- 5,11 ---- X # a few defines for testing purposes; if you test in the current X # directory the paths needn't be absolute X # the path of indir will be used in #! lines -> max. 32 characters for X! # #! + name + space (or tab) + mandatory option X PATH_OF_INDIR = ./indir X TEST_DIRECTORY = . X X*************** X*** 21,32 **** X $(CC) -O -o indir $(OBJS) X X exec_ok: X! exec_test X X test: exec_ok X test -f tests_made || PATH_OF_INDIR=$(PATH_OF_INDIR) \ X! TEST_DIRECTORY=$(TEST_DIRECTORY) make_tests X! do_tests X X lint: X lint $(SRCS) X--- 21,32 ---- X $(CC) -O -o indir $(OBJS) X X exec_ok: X! ./exec_test X X test: exec_ok X test -f tests_made || PATH_OF_INDIR=$(PATH_OF_INDIR) \ X! TEST_DIRECTORY=$(TEST_DIRECTORY) ./make_tests X! ./do_tests X X lint: X lint $(SRCS) X*** old/do_tests Sat Feb 2 07:18:58 1991 X--- do_tests Sat Feb 2 07:23:38 1991 X*************** X*** 1,15 **** X #!/bin/sh X for i in wrong.* ok.* X do X! echo '' >&2 X! echo "$ ls -l $i" >&2 X! ls -l $i >&2 X! echo "$ cat $i" >&2 X! cat $i >&2 X! echo $ X! echo -n "-- hit return to run $i: " >&2 X read x X! $i file_argument X! echo -n "-- hit return to continue: " >&2 X read x X done X--- 1,18 ---- X #!/bin/sh X+ X for i in wrong.* ok.* X do X! echo '' X! echo "$ ls -l $i" X! ls -l $i X! echo "$ cat $i" X! cat $i X! echo "$ " X! echo -n "-- hit return to run $i: " X read x X! ./$i file_argument X! echo -n "-- hit return to continue: " X read x X done X+ X+ exit 0 X*** old/indir.c Sat Feb 2 07:19:48 1991 X--- indir.c Sat Feb 2 07:23:38 1991 X*************** X*** 1,4 **** X! static char sccsid[] = "@(#)indir.c 2.3 90/03/31 Maarten Litmaath"; X X /* X * indir.c X--- 1,5 ---- X! static char sccsid[] = X! "@(#)indir.c 2.4 91/02/02 Maarten Litmaath @ CS, VU, Amsterdam"; X X /* X * indir.c X*************** X*** 145,150 **** X--- 146,152 ---- X execvp(Interpreter, Newargv); X X error(E_exec, Prog, Interpreter, File, geterr()); X+ /* NOTREACHED */ X } X X X*************** X*** 270,276 **** 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--- 272,278 ---- X char *f; X { X struct stat stbuf; X! int xmask, pid, groups_member(); X uid_t uid; X gid_t gid; X static int status = -1, checked = 0; X*************** X*** 301,308 **** 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--- 303,311 ---- 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! groups_member(st->st_gid) ? X_GRP : X X_OTH; X /* X * Can the invoker really execute the file we're reading from? X*************** X*** 330,335 **** X--- 333,355 ---- X while (wait(&w) != pid) X ; X return ok(w) ? status = 0 : -1; X+ } X+ X+ X+ static int groups_member(gid) X+ int gid; X+ { X+ register X+ int *p, ngroups; X+ int groups[NGROUPS]; X+ X+ if ((ngroups = getgroups(NGROUPS, groups)) < 0) X+ return 0; X+ p = groups; X+ while (--ngroups >= 0) X+ if (*p++ == gid) X+ return 1; X+ return 0; X } X X X*** old/setuid.txt Sat Feb 2 07:19:49 1991 X--- setuid.txt Sat Feb 2 07:23:38 1991 X*************** X*** 1,6 **** X--- 1,7 ---- X Setuid Shell Scripts X -------------------- X how to get them safe X+ version 1.1 X X Maarten Litmaath X (maart@cs.vu.nl) X*************** X*** 42,54 **** X Yes, one needs write permission somewhere on the same device, if one's X operating system doesn't support symbolic links. X X! What about the csh command interpreter? Well, 4.2BSD provides us with a csh X! which has a NEW option: "-b"! Its goal is to avoid just the thing described X! above: the mnemonic for `b' is `break'; this option prevents following X! arguments of an exec of /bin/csh from being interpreted as options... X! The csh refuses to run a setuid shell script unless the option is present... X Scheme: X! #!/bin/csh -b X ... X X execl("-i", "unimportant", (char *) 0); X--- 43,54 ---- X Yes, one needs write permission somewhere on the same device, if one's X operating system doesn't support symbolic links. X X! What about the bare `-' option present in modern versions of the Bourne X! shell? Its goal is to avoid just the thing described above: this option X! prevents following arguments of an exec of /bin/sh from being interpreted X! as options... X Scheme: X! #!/bin/sh - X ... X X execl("-i", "unimportant", (char *) 0); X*************** X*** 58,64 **** X X setuid(0); X setgid(0); X! execl("/bin/csh", "csh", "-b", "-i", (char *) 0); X X And indeed the contents of the file "-i" are executed! X However, there's still another bug hidden, albeit not for long! X--- 58,64 ---- X X setuid(0); X setgid(0); X! execl("/bin/sh", "sh", "-", "-i", (char *) 0); X X And indeed the contents of the file "-i" are executed! X However, there's still another bug hidden, albeit not for long! X*************** X*** 82,95 **** X currently the total length of shell + argument mustn't exceed 32 chars X (easily fixed); X 2) X! 4.[23]BSD csh is expecting a `-b' flag as the first argument, instead X! of the full path (easily fixed); X 3) X the interpreter gets an extra argument; X 4) X the difficulty of maintaining setuid shell scripts increases - if one X moves a script, one shouldn't forget to edit it... - editing in turn X! could turn off the setuid bits, so one shouldn't forget to chmod(1) X the file `back'... - conceptually the solution above isn't `elegant'. X X How does indir(1) tackle the problems? The script to be executed will look X--- 82,95 ---- X currently the total length of shell + argument mustn't exceed 32 chars X (easily fixed); X 2) X! when executing a setuid script, 4.[23]BSD csh is expecting a `-b' flag X! as the first argument, instead of the full path (easily fixed); X 3) X the interpreter gets an extra argument; X 4) X the difficulty of maintaining setuid shell scripts increases - if one X moves a script, one shouldn't forget to edit it... - editing in turn X! could turn off the setuid bit, so one shouldn't forget to chmod(1) X the file `back'... - conceptually the solution above isn't `elegant'. X X How does indir(1) tackle the problems? The script to be executed will look + END-OF-FILE indir2.4diffs chmod 'u=rw,g=r,o=r' 'indir2.4diffs' set `wc -c 'indir2.4diffs'` count=$1 case $count in 6805) :;; *) echo 'Bad character count in ''indir2.4diffs' >&2 echo 'Count should be 6805' >&2 esac exit 0