Usenet 1994 October/usenetsourcesnewsgroupsinfomagicoctober1994disk2.iso/unix/volume22/indir.pch

Text File | 1990-06-07 | 14KB | 393 lines

Subject:  v22i010:  Patch to indir -- safely execute (set[ug]id) scripts
Newsgroups: comp.sources.unix
Approved: [e-mail]
X-Checksum-Snefru: b3f85d0f 8d8c5d5a cc236020 fadee82a

Submitted-by: Maarten Litmaath <[e-mail]>
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 <[e-mail]> 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:
                [e-mail], 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