doug@letni.UUCP (Doug Davis) (12/15/88)
Before you flame, this got posted to news.admin because thats
where the original message about this showed up. besides this
is small.
Attached is a version of /bin/mkdir that should eliminate the
problem with the race condition that someone can take advantage
of to cause a major security hole. Machines that have the
mkdir() function call, most anything based on 4.2 BSD or later,
will not need this program. Everyone else that I know of
should want this.
For security reasons I have elected not to describe the problem with /bin/mkdir
fully, just suffice it to say that I have tested it on 11 differen't
architectures and the "bug" existed on all of them. If your /bin/mkdir
program is setuid root, you too probably have this bug as well.
This mkdir first makes a directory to play in, which is owned by root
and is mode 000. It is made in the same directory in which the
user is requesting his directory. In this "secure" directory, to
which the user allegedly has no access, the mknod(), chown(), and
links for `.' and `..' are performed. The new directory is then linked
into place. Finally, the "secure" directory is removed. Yes, there
is a bit more overhead, but a much more secure program is worth it.
As usual, I will accept mail, suggestions, comments, etc will
be appreciated. Flames will be ignored. If anyone can poke security holes
in this code I would really like to hear about it.
BTW: I know the calls to rand() are not really needed. They just make
it more fun for someone trying to defeat the code.
Doug Davis
--
Lawnet
1030 Pleasent Valley Lane.
Arlington Texas 76015
817-467-3740
{ sys1.tandy.com, motown!sys1, uiucuxc!sys1, killer!texbell } letni!doug
"Talk about holes in UNIX, geeze thats nothing compaired with the security
problems in the ship control programs of StarFleet."
#! /bin/sh
# This is a shell archive. Remove anything before this line, then unpack
# it by saving it into a file and typing "sh file". To overwrite existing
# files, type "sh file -c". You can also feed this as standard input via
# unshar, or by typing "sh <file", e.g.. If this archive is complete, you
# will see the following message at the end:
# "End of shell archive."
# Contents: Makefile mkdir.c
# Wrapped by doug@letni on Thu Dec 15 01:28:50 1988
# { sys1.tandy.com, motown!sys1, uiucuxc!sys1, killer!texbell } letni!doug
PATH=/bin:/usr/bin:/usr/ucb ; export PATH
if test -f 'Makefile' -a "${1}" != "-c" ; then
echo shar: Will not clobber existing file \"'Makefile'\"
else
echo shar: Extracting \"'Makefile'\" \(510 characters\)
sed "s/^X//" >'Makefile' <<'END_OF_FILE'
X# What kind of strrchr do we have?
X# 'strrchr' for most newer unix's, 'rindex' for earlier editions
X# STTRCHR = -DSTRRCHR=rindex
STRRCHR = -DSTRRCHR=strrchr
X# do you have a rand() function call?
X# If you don't have one, comment out the line below
RAND = -DRAND
X
X# for debugging,
X# -DDEBUG
X
DEFINES = $(STRRCHR) $(DEBUG) $(RAND)
SHELL = /bin/sh
CC = /bin/cc
CFLAGS = -O $(DEFINES)
LDFLAGS = -n -s
X
all: mkdir
X
mkdir.o: mkdir.c
X $(CC) $(CFLAGS) mkdir.c -c
X
mkdir: mkdir.o
X $(CC) $(LDFLAGS) mkdir.o -o mkdir
END_OF_FILE
if test 510 -ne `wc -c <'Makefile'`; then
echo shar: \"'Makefile'\" unpacked with wrong size!
fi
# end of 'Makefile'
fi
if test -f 'mkdir.c' -a "${1}" != "-c" ; then
echo shar: Will not clobber existing file \"'mkdir.c'\"
else
echo shar: Extracting \"'mkdir.c'\" \(7645 characters\)
sed "s/^X//" >'mkdir.c' <<'END_OF_FILE'
X/*
X * Secure mkdir program, solves that nasty race problem...
X *
X * 13 December 1988 Doug Davis doug@lenti.lawnet.com
X * and John Elliot IV iv@trsvax.tandy.com
X *
X *
X * Theory of operation:
X * This mkdir first makes a directory to play in, which is
X * owned by root and is mode 000. It is made in the same
X * directory in which the user is requesting his directory.
X * In this "secure" directory, to which the user allegedly
X * has no access, the mknod(), chown(), and links for `.'
X * and `..' are performed. The new directory is then linked
X * into place. Finally, the "secure" directory is removed.
X *
X * This code copyright 1988 by Doug Davis (doug@letni.lawnet.com)
X * You are free to modify, hack, fold, spindle, duplicate, pass-along
X * give-away, publish, transmit, or mutlate this code in any maner,
X * provided that you give credit where credit is due and don't pretend
X * that you wrote it.
X *
X * If you do my lawyers (and I have a lot of lawyers) will teach you a lesson
X * or two in copyright law that you will never ever forget.
X */
X
X#define MAXPATHLEN 128 /* maximum reasonanble path length */
X
X#include <sys/types.h>
X#include <signal.h>
X#include <sys/stat.h>
X#ifdef DEBUG
X# include <stdio.h>
X#else /*DEBUG*/
X# define NULL ((char *) 0)
X#endif /*DEBUG*/
X
X#define MKNODE 1
X#define LINK 2
X
char *Malloc_Failed = "malloc() failed.";
char *Doesnt_Exist = " does not exist.";
char *Cannot_Access = "cannot access ";
char *Already_Exist = " already exists.";
char *Secure_Failed = "makedir secure parent failed ";
char *Couldnt_Link = "Couldn't link to ";
char *Mkdir_Failed = "makedir failed ";
char *Chown_Failed = "chown() failed ";
X
extern char *STRRCHR();
extern char *malloc();
X
extern int errno;
extern int getpid();
X
extern unsigned short getgid();
extern unsigned short getuid();
X
X#ifdef RAND
extern int rand();
X#else /*RAND*/
extern int getppid();
X#endif /*RAND*/
X
extern long time();
X
char *Progname;
X
main(argc, argv)
int argc;
char *argv[];
X{
X Progname = argv[0];
X errno = 0;
X
X if (argc < 2) {
X print("Usage: ");
X print(Progname);
X print(" directory_name [ ... directory_name ]\n");
X exit(0);
X }
X
X /* Catch those nasty signals that could cause us
X * to mess up the filesystem */
X swat_sigs();
X
X while (--argc)
X md(*++argv); /* make each directory */
X
X exit(errno);
X}
X
X
md(s)
char *s;
X{
X char *basename, *parent, *fullname;
X char securename[MAXPATHLEN], securedir[MAXPATHLEN];
X long snum;
X unsigned short myuserid, mygroupid;
X struct stat sanity;
X
X /* find out who I really am */
X myuserid = getuid();
X mygroupid = getgid();
X
X /* set up the pseudo-RANDom number generation system */
X#ifndef RAND
X srand(getpid());
X#endif /*RAND*/
X
X /* see if we are explicit or indirect */
X basename = STRRCHR(s, '/');
X if (basename == (char *) NULL) {
X fullname = malloc(strlen(s)+1);
X if (fullname == (char *) NULL)
X error(Malloc_Failed, NULL, errno);
X parent = malloc(2);
X if (parent == (char *) NULL)
X error(Malloc_Failed, NULL, errno);
X parent[0] = '.';
X parent[1] = '\0';
X strcpy(fullname, s);
X basename = s;
X } else {
X fullname = malloc(strlen(s)+1);
X if (fullname == (char *) NULL)
X error(Malloc_Failed, NULL, errno);
X strcpy(fullname, s);
X *basename = '\0';
X basename++;
X parent = malloc(strlen(s) + 3);
X if (parent == (char *) NULL)
X error(Malloc_Failed, NULL, errno);
X strcpy(parent, s);
X strcat(parent, "/.");
X }
X
X /* Generate the secure names ... */
X do {
X /* round and round we go where we stop depends on
X * the non-existance of securedir */
X snum = time((int *) 0);
X#ifdef RAND
X sprintf(securedir, "%s/%ld", parent, snum - (long)rand());
X sprintf(securename, "%s/%ld", securedir, snum + (long)rand());
X#else /*RAND*/
X sprintf(securedir, "%s/%ld", parent, snum - (long)getppid());
X sprintf(securename, "%s/%ld", securedir, snum + (long)getppid());
X snum += (long)getpid();
X#endif /*RAND*/
X } while (stat(securedir, &sanity) == 0);
X
X#ifdef DEBUG
X /* spill the beans .. */
X printf("parent == %s\n", parent);
X printf("basename == %s\n", basename);
X printf("fullname == %s\n", fullname);
X printf("securedir == %s\n", securedir);
X printf("securename == %s\n", securename);
X fflush(stdout);
X#endif /*DEBUG*/
X
X /* lets see if our parent directory is around... */
X if ((stat(parent, &sanity)) != 0)
X error(parent, Doesnt_Exist, 0);
X
X /* find out if we can write here */
X if (canIwrite(&sanity, myuserid, mygroupid) != 0)
X error(Cannot_Access, parent, 0);
X
X /* find out if we are going to stomp on something.. */
X if ((stat(fullname, &sanity)) == 0)
X error(fullname, Already_Exist, 0);
X
X /* make secure parent directory (note the mode of 0) */
X if (makedir(parent, securedir, 0) > 0)
X error(Secure_Failed, securedir, errno);
X
X /* now make our directory underneath it */
X if (makedir(parent, securename, 0777) > 0)
X error(Mkdir_Failed, securedir, errno);
X
X /* do that eerie little chown() thats the "root" of all our problems */
X if (chown(securename, myuserid, mygroupid) != 0)
X error(Chown_Failed, securename, errno);
X
X /* do a quick sanity check, just to annoy someone trying, unsccessfully
X * I might add, to trick mkdir into chowning something it shouldn't.. */
X if ((stat(fullname, &sanity)) == 0) {
X /* what happend? this wasn't here a couple of functions ago.. */
X unlink(securename);
X rmdir(securedir);
X error(fullname, Already_Exist, 0);
X }
X
X /* okay, put it where it belongs */
X if ((link(securename, fullname)) < 0)
X error(Couldnt_Link, fullname, errno);
X
X /* remove all our rubbish, and tidy everything up.. */
X unlink(securename);
X rmdir(securedir);
X if (parent != (char *) NULL)
X free(parent);
X if (fullname != (char *) NULL)
X free(fullname);
X return(0);
X}
X
makedir(parent, dir, mode)
char *parent, *dir;
int mode;
X{
X char dotdot[MAXPATHLEN];
X
X#ifdef DEBUG
X printf("mkdir(%s, %s)\n", parent, dir);
X fflush(stdout);
X#endif /*DEBUG*/
X
X /* put the node together */
X if ((mknod(dir, S_IFDIR | mode, 0)) < 0)
X return (MKNODE);
X
X /* make dot */
X strcpy(dotdot, dir);
X strcat(dotdot, "/.");
X if ((link(dir, dotdot)) < 0)
X return (LINK);
X
X /* make dotdot */
X strcat(dotdot, ".");
X if ((link(parent, dotdot)) < 0)
X return (LINK);
X
X return (0);
X}
X
rmdir(dir)
char *dir;
X{
X char dots[MAXPATHLEN];
X
X#ifdef DEBUG
X printf("rmdir(%s)\n", dir);
X fflush(stdout);
X#endif /*DEBUG*/
X
X strcpy(dots, dir);
X strcat(dots, "/.");
X
X /* unlink(".") */
X if (unlink(dots) < 0)
X return (LINK);
X
X /* unlink("..") */
X strcat(dots, ".");
X if (unlink(dots) < 0)
X return (LINK);
X
X /* unlink the directory itself */
X if (unlink(dir) < 0)
X return (LINK);
X
X return (0);
X}
X
print(s)
char *s;
X{
X write(2, s, strlen(s));
X}
X
error(s1, s2, err)
char *s1, *s2;
int err;
X{
X write(2, Progname, strlen(Progname));
X write(2, ": ", 2);
X write(2, s1, strlen(s1));
X errno = err;
X if (s2 != NULL)
X write(2, s2, strlen(s2));
X if (err != 0)
X perror(" ");
X else
X write(2, "\n", 1);
X exit(errno);
X}
swat_sigs()
X{
X register int i;
X
X for (i=SIGHUP; i<=NSIG ; i++)
X signal(i, SIG_IGN); /* bye-bye */
X}
canIwrite(stbuff, uid, gid)
register struct stat *stbuff;
register unsigned short uid, gid;
X{
X /* we let root get away with anything... */
X if (uid == 0)
X return(0);
X
X /* can I write in it as an OWNER ? */
X if (uid == stbuff->st_uid && stbuff->st_mode & 0200)
X return(0);
X
X /* okay, so how about as a GROUP ? */
X if (gid == stbuff->st_gid && stbuff->st_mode & 0020)
X return(0);
X
X /* alright, how about an OTHER ? */
X if (stbuff->st_mode & 0002)
X return(0);
X
X /* okay, so I can't write here.. */
X return(-1);
X}
X#ifdef DEBUG
unlink(s)
char *s;
X{
X printf("Unlink(%s)\n", s);
X fflush(stdout);
X}
X#endif /*DEBUG*/
END_OF_FILE
if test 7645 -ne `wc -c <'mkdir.c'`; then
echo shar: \"'mkdir.c'\" unpacked with wrong size!
fi
chmod +x 'mkdir.c'
# end of 'mkdir.c'
fi
echo shar: End of shell archive.
exit 0ddl@husc6.harvard.edu (Dan Lanciani) (12/17/88)
The proposed mkdir replacement does not solve the problem. I
do not know if it introduces additional problems of its own, but I
would not recommend running it since the gain in security is minimal.
I will not describe in detail the variation required to subvert the
mkdir replacement, but consider the interval immediately before its
chown() call. I will outline a mechanism for creating a mkdir program
which can at worst be tricked into changing the ownership of a corrupt
(0-length) directory to which the (ab)user already has write/rename
access or to which s/he can (sym)link. Unfortunately, this mechanism
requires that "" refer to the current working directory and this
feature/bug has been eliminated from System V.
1. Create (mknod) the desired directory with restrictive permissions.
(At this point, the user may delete the new directory and
move into place one of his own in an attempt to acquire ownership.)
2. Change working directory to the created directory (chdir).
3. Obtain the size of the current working directory (stat). If it
is non-zero, someone is trying to spoof the program. Exit, make
a log entry, or whatever.
4. Link the current working directory to "." (link("", ".")). If this
fails, something is seriously wrong. Exit.
5. Link the parent to "..". Exit if it fails.
6. Change the ownership of the current working directory (chown("",...)).
Note that since this procedure changes the current working directory
of the process, a multiple-argument mkdir must fork() n - 1 times
to make n directories. It is inadvisable to attempt to return to
the previous current working directory in order to avoid the fork().
Ambitious security analysts might want to examine rmdir and
suid versions of mv for related, but less serious, problems.
Dan Lanciani
ddl@harvard.*doug@letni.UUCP (12/18/88)
In article <851@husc6.harvard.edu> ddl@husc6.harvard.edu (Dan Lanciani) writes: > > The proposed mkdir replacement does not solve the problem. I >do not know if it introduces additional problems of its own, but I >would not recommend running it since the gain in security is minimal. >I will not describe in detail the variation required to subvert the >mkdir replacement, but consider the interval immediately before its >chown() call. Before you go bashing peoples code with induendos about how it supposably does not work, why don't you do the author(s) a favor and send them either private mail, or call them on the phone. Since its very easy to say %s program doesn't work and you shouldn't run it, and I won't go into why. It makes me wonder if either my program really does have a problem, in which case I do need to know, or you didn't pay enough attention to how the program ran. Especially the area immediately before and right after the chown() call, look at what directory is getting chown'ed and what permissions it's parent has. So my advice to the net is to make your own discisions on what to run the original /bin/mkdir Which does have a problem. My mkdir which might have a problem, or it might not, but either way is more secure than /bin/mkdir. I really don't think that there is a problem with even posting a way to get around my mkdir, since its not the standard mkdir program, and undoubtably will not have the same security problems. doug -- Lawnet 1030 Pleasent Valley Lane. Arlington Texas 76015 817-467-3740 { sys1.tandy.com, motown!sys1, uiucuxc!sys1, killer!texbell } letni!doug "Talk about holes in UNIX, geeze thats nothing compaired with the security problems in the ship control programs of StarFleet."
ddl@husc6.harvard.edu (Dan Lanciani) (12/18/88)
In article <10048@merch.TANDY.COM>, doug@letni.UUCP writes: | In article <851@husc6.harvard.edu> ddl@husc6.harvard.edu (Dan Lanciani) writes: | > | > The proposed mkdir replacement does not solve the problem. I | >do not know if it introduces additional problems of its own, but I | >would not recommend running it since the gain in security is minimal. | >I will not describe in detail the variation required to subvert the | >mkdir replacement, but consider the interval immediately before its | >chown() call. | Before you go bashing peoples code with induendos[sic] about how it supposably | does not work, why don't you do the author(s) a favor and send them | either private mail, or call them on the phone. Since its very | easy to say %s program doesn't work and you shouldn't run it, and I won't | go into why. I didn't say that the program doesn't work. I merely said that it does not solve the problem that it sets out to solve and that I do not *know* that it does not create additional problems of its own. Considering the sensitivity of the issue and that fact that the replacement mkdir was made public, it seems appropriate that criticism of it also be made public. You will also probably be better off when someone decides to sue you for providing a "Secure mkdir program, solves that nasty race problem..." that really isn't/doesn't.:) I.e., you can say s/he had some warning... | It makes me wonder if either my program really does have | a problem, in which case I do need to know, or you didn't pay enough | attention to how the program ran. Especially the area immediately before | and right after the chown() call, look at what directory is getting | chown'ed and what permissions it's parent has. I'm not sure what you mean by "need to know," but since the problem can't be fixed using the approach in the proposed replacement it wouldn't help. | So my advice to the net is to make your own discisions[sic] on what to run | the original /bin/mkdir Which does have a problem. My mkdir which | might have a problem, or it might not, but either way is more secure | than /bin/mkdir. Alternately, someone on the net might be able to write a secure mkdir based on the outline I presented. | I really don't think that there is a problem with even posting | a way to get around my mkdir, since its not the standard mkdir | program, and undoubtably will not have the same security problems. On the contrary, it has exactly the same problem as the standard mkdir and the techniques required to exploit that problem are just those that would make it even easier (i.e., quicker, less time sensitive) to subvert the standard mkdir. Dan Lanciani ddl@harvard.* PS: I don't read news.admin, so I may miss future followups...
jfh@rpp386.Dallas.TX.US (The Beach Bum) (12/20/88)
In article <851@husc6.harvard.edu> ddl@husc6.harvard.edu (Dan Lanciani) writes: > The proposed mkdir replacement does not solve the problem. I >do not know if it introduces additional problems of its own, but I >would not recommend running it since the gain in security is minimal. I wouldn't go THAT far. The fix is very simple and the resultant code should be significantly more secure. However, Doug's mkdir is no more secure than the AT&T version. Only the method required to break it is different, and yes it is breakable. Almost as easily so. To break in, the following behavior is required. The unlink() call is needed, along with mv(1) being able to rename a directory. In this case, a C program is really needed so the directory can be read. This is a plus since using nice(2) increases the window during which the directory can be cracked. You can even get around the need for a C program. Simply have ls tell you what files are there ... > I will outline a mechanism for creating a mkdir program >which can at worst be tricked into changing the ownership of a corrupt >(0-length) directory to which the (ab)user already has write/rename >access or to which s/he can (sym)link. Unfortunately, this mechanism >requires that "" refer to the current working directory and this >feature/bug has been eliminated from System V. This is bunk. A completely secure version can be created using the outline provided by Doug. The '' feature is not required, the '.' link created inside of the `secure' directory is more than addequate since only root would be able to do anything to that link, once created. And, once inside the secure directory, the integrity of the entire directory can be tested. This should make it impossible to spoof mkdir. >1. Create (mknod) the desired directory with restrictive permissions. So far, so good. > (At this point, the user may delete the new directory and > move into place one of his own in an attempt to acquire ownership.) Yes, this is the flaw with the real mkdir and Doug's as well. >2. Change working directory to the created directory (chdir). This is part of the fix. You can't allow the secure directory to be accessed from outside of the secure directory by the program. That directory itself could be replaced with a `bad guy' version. >3. Obtain the size of the current working directory (stat). If it > is non-zero, someone is trying to spoof the program. Exit, make > a log entry, or whatever. Not needed, and even still, this is spoofable [ I think ... ] in the absense of the '' bug. >4. Link the current working directory to "." (link("", ".")). If this > fails, something is seriously wrong. Exit. This requirement can be gotten around by verifing you have the correct ".." link in your directory along with the correct "." link. Hint: fstat the file descriptor you get after opening the directories and HOLDING them open. You will need 4 open files, the parent, the secure directory, and the . and .. entries in the secure directory. Test those to insure the links are to the correct files. Once the directory is secure, you can do all of the other tinkering inside of the secure directory without fear of compromise. >6. Change the ownership of the current working directory (chown("",...)). Would be nice if you could do it, no? However, we have validated our "." entry after 4), so we can chown (".", ...); THAT works. >Note that since this procedure changes the current working directory >of the process, a multiple-argument mkdir must fork() n - 1 times >to make n directories. It is inadvisable to attempt to return to >the previous current working directory in order to avoid the fork(). Again, we have validated our ".." entry to point towards our correct parent directory. There is no reason it should not work. > Ambitious security analysts might want to examine rmdir and >suid versions of mv for related, but less serious, problems. Ambitious nay-sayers should always call the programmer being brought to task and have a nice long conversation. Or do us a favor and point out the flaw explicitly. To summarize, Doug's mkdir has the same weakness as the AT&T version. It took 34 seconds to break this site [ after altering a few directories first ... ], after writing a suitable script it should take about the same amount of time using Doug's. Using a C program would take less time since the window of oppurtunity can be narrowed, and more attempts can be made to locate the `secure' directory. This is the only change required. Oh yes, I may do a mkdir of my own and post it. I suspect Doug will beat me to it ... Sorry, Doug, you better get ready on that 'C' key ;-) -- John F. Haugh II +-Quote of the Week:------------------- VoiceNet: (214) 250-3311 Data: -6272 |"Unix doesn't have bugs, InterNet: jfh@rpp386.Dallas.TX.US | Unix is a bug" UucpNet : <backbone>!killer!rpp386!jfh +-- -- author forgotten --
jfh@rpp386.Dallas.TX.US (The Beach Bum) (12/20/88)
In article <10115@rpp386.Dallas.TX.US> jfh@rpp386.Dallas.TX.US (The Beach Bum) writes: I outlined the security problems with the recently posted mkdir from Doug Davis. As a brief refresher, that mkdir had the same problem as the version from AT&T, only slightly different. The vulnerable point was after the makedir() call and before the chown() call. A bogus file could be substituted in there by unlink(2)'ing the secure directory and mv(1)'ing in a pre-built directory containing the link to the file to be chown(2)'d. Below I have a set of patches which corrects that particular problem. Doug is on the phone right NOW!!! and he is going to post an official patch containing a number of fixes. USE THAT when it comes out, but you can play with this in the interim. The fix is very simple [ as I said it would be ... ]. It does the chown on "securename/./.", which insures that "." is a directory. Since only root can link directories, the bad guy would have had to have been root to have created the bogus link which he wanted the ownership of changed. - John. -- *** o.mkdir.c Mon Dec 19 13:02:27 1988 --- mkdir.c Mon Dec 19 14:05:59 1988 *************** *** 95,101 char *s; { char *basename, *parent, *fullname; ! char securename[MAXPATHLEN], securedir[MAXPATHLEN]; long snum; unsigned short myuserid, mygroupid; struct stat sanity; --- 95,101 ----- char *s; { char *basename, *parent, *fullname; ! char securename[MAXPATHLEN], securedir[MAXPATHLEN], securenamedot[MAXPATHLEN]; long snum; unsigned short myuserid, mygroupid; struct stat sanity; *************** *** 144,149 #ifdef RAND sprintf(securedir, "%s/%ld", parent, snum - (long)rand()); sprintf(securename, "%s/%ld", securedir, snum + (long)rand()); #else /*RAND*/ sprintf(securedir, "%s/%ld", parent, snum - (long)getppid()); sprintf(securename, "%s/%ld", securedir, snum + (long)getppid()); --- 144,150 ----- #ifdef RAND sprintf(securedir, "%s/%ld", parent, snum - (long)rand()); sprintf(securename, "%s/%ld", securedir, snum + (long)rand()); + sprintf(securenamedot, "%s/./.", securename); #else /*RAND*/ sprintf(securedir, "%s/%ld", parent, snum - (long)getppid()); sprintf(securename, "%s/%ld", securedir, snum + (long)getppid()); *************** *** 147,152 #else /*RAND*/ sprintf(securedir, "%s/%ld", parent, snum - (long)getppid()); sprintf(securename, "%s/%ld", securedir, snum + (long)getppid()); snum += (long)getpid(); #endif /*RAND*/ } while (stat(securedir, &sanity) == 0); --- 148,154 ----- #else /*RAND*/ sprintf(securedir, "%s/%ld", parent, snum - (long)getppid()); sprintf(securename, "%s/%ld", securedir, snum + (long)getppid()); + sprintf(securenamedot, "%s/./.", securename); snum += (long)getpid(); #endif /*RAND*/ } while (stat(securedir, &sanity) == 0); *************** *** 158,163 printf("fullname == %s\n", fullname); printf("securedir == %s\n", securedir); printf("securename == %s\n", securename); fflush(stdout); #endif /*DEBUG*/ --- 160,166 ----- printf("fullname == %s\n", fullname); printf("securedir == %s\n", securedir); printf("securename == %s\n", securename); + printf("securenamedot == %s\n", securenamedot); fflush(stdout); #endif /*DEBUG*/ *************** *** 182,188 error(Mkdir_Failed, securedir, errno); /* do that eerie little chown() thats the "root" of all our problems */ ! if (chown(securename, myuserid, mygroupid) != 0) error(Chown_Failed, securename, errno); /* do a quick sanity check, just to annoy someone trying, unsccessfully --- 185,191 ----- error(Mkdir_Failed, securedir, errno); /* do that eerie little chown() thats the "root" of all our problems */ ! if (chown(securenamedot, myuserid, mygroupid) != 0) error(Chown_Failed, securename, errno); /* do a quick sanity check, just to annoy someone trying *************** *** 185,192 if (chown(securename, myuserid, mygroupid) != 0) error(Chown_Failed, securename, errno); ! /* do a quick sanity check, just to annoy someone trying, unsccessfully ! * I might add, to trick mkdir into chowning something it shouldn't.. */ if ((stat(fullname, &sanity)) == 0) { /* what happend? this wasn't here a couple of functions ago.. */ unlink(securename); --- 188,195 ----- if (chown(securenamedot, myuserid, mygroupid) != 0) error(Chown_Failed, securename, errno); ! /* do a quick sanity check, just to annoy someone trying ! * to trick mkdir into chowning something it shouldn't.. */ if ((stat(fullname, &sanity)) == 0) { /* what happend? this wasn't here a couple of functions ago.. */ unlink(securename); -- John F. Haugh II +-Quote of the Week:------------------- VoiceNet: (214) 250-3311 Data: -6272 |"Unix doesn't have bugs, InterNet: jfh@rpp386.Dallas.TX.US | Unix is a bug" UucpNet : <backbone>!killer!rpp386!jfh +-- -- author forgotten --
wcs@skep2.ATT.COM (Bill.Stewart.[ho95c]) (12/22/88)
nice(-255); /* always win race condition - fixes security bug */
/* nice(-255) is not very nice, but root has its privileges */
/* works with official mkdir and Doug's */
The original articles in news.admin pointed out a race
condition in the /bin/mkdir used by V7 and System V
older releases. (mkdir runs setuid root, mknod's a
directory then chowns it to getuid() - if you're fast you
can rmdir the directory and ln /etc/passwd, especially if
you nice -19 mkdir.) Doug Davis posted a public-domain
mkdir which he claimed fixed the problem. Other people have
pointed out that it helps, but doesn't prevent the race
condition, and it's free source for people who only have
binary systems.
Put a nice(-255) as the first line of your mkdir, and you'll
"never" lose the race!! The race condition worked, because
on a loaded system, you could guarantee that sometimes your
rm-and-link hack would happen between the mknod() and the chown(),
as long as you ran the mkdir niced so it had a low priority.
But remember that you're setuid root, so you don't have to be
nice() just because some luser told you to be.
Nice(-255) truncates to the nastiest priority a user-level
process can have, no matter how nice() the mkdir had
originally been.
Under System V, nice() returns -1 if it can't get the nice value you
asked for; my 4.1BSD manual doesn't say what it does. It's good
programming practice to do
if (nice(-255) == -1) { perror("mkdir fails:"); exit(1); }
but it really can't happen since you're root.
====== commentary =======
The security bug was REAL, for systems using a setuid-root mkdir
command instead of mkdir() system call. SVR3.0 and 4.2BSD both
have mkdir(), but earlier System V, V7, and 4.1BSD all use the
older approach. The nice(-255) isn't a 100% cure, but I've been
unable to break any of our systems since we installed it, and
it doesn't break anything.
Multiprocessor systems may still have some risk.
We looked at some alternatives like Doug's and Dan's that
might have the potential to cause trouble - mkdir is a critical
enough program that I'd want to thoroughly test anything that
wasn't really simple before I used it, but this change is as
transparent as any I could think of. We may still try some more
hacks, but this is cheap, easy, and works for uniprocessors.
--
# Thanks;
# Bill Stewart, AT&T Bell Labs 2G218 Holmdel NJ 201-949-0705 ho95c.att.com!wcs
#
# News. Don't ask me about News.ddl@husc6.harvard.edu (Dan Lanciani) (12/22/88)
In article <379@skep2.ATT.COM>, wcs@skep2.ATT.COM (Bill.Stewart.[ho95c]) writes: | nice(-255); /* always win race condition - fixes security bug */ | /* nice(-255) is not very nice, but root has its privileges */ | /* works with official mkdir and Doug's */ | | The original articles in news.admin pointed out a race | condition in the /bin/mkdir used by V7 and System V | older releases. (mkdir runs setuid root, mknod's a | directory then chowns it to getuid() - if you're fast you | can rmdir the directory and ln /etc/passwd, especially if | you nice -19 mkdir.) Doug Davis posted a public-domain | mkdir which he claimed fixed the problem. Other people have | pointed out that it helps, but doesn't prevent the race | condition, and it's free source for people who only have | binary systems. | | Put a nice(-255) as the first line of your mkdir, and you'll | "never" lose the race!! The race condition worked, because | on a loaded system, you could guarantee that sometimes your | rm-and-link hack would happen between the mknod() and the chown(), | as long as you ran the mkdir niced so it had a low priority. Unfortunately, this analysis is incorrect. The real window occurs while the mkdir process is blocked on disk I/O to, e.g., look up elements of the path name of the file to chown(). No matter how good a priority the process has, other processes will run while it waits for the disk. In fact, it is doubtful that you would ever see the mkdir attack succeed if it actually required that the system preempt the mkdir process "between" the mknod() and the chown(). Tests with a well-crafted mkdir-breaker program showed that success was nearly independent of process priority. Incidentally, the fix proposed by jfh@rpp386 (using dir/./. as the target of the chown()) doesn't help either. It was a good try (and happened to be included in the mkdir test mentioned above) but breaks down since link() itself is not atomic. Dan Lanciani ddl@harvard.*
jfh@rpp386.Dallas.TX.US (The Beach Bum) (12/22/88)
In article <379@skep2.ATT.COM> wcs@skep2.UUCP (46323-Bill.Stewart.[ho95c],2G218,x0705,) writes: >nice(-255); /* always win race condition - fixes security bug */ > /* nice(-255) is not very nice, but root has its privileges */ > /* works with official mkdir and Doug's */ Nope, this fails. Consider - nice() does not insure you are always first, it only insures that you are preferred. After some period of execution, the priority of the process will drop low enough for the user to slip in. Instead of doing a single directory per mkdir, stuff the command line FULL of directories. Also, the lowest NICE is 0. The default NICE is 20. This only means that proc.p_cpu for your mkdir process needs to be 20 more than p_cpu for the bad guys process. One full second of execution should do this. Once that is accomplished, the bad guy should be able to slip in between. A C program may be needed to get the timing information correct, but it should be VERY possible. -- John F. Haugh II +-Quote of the Week:------------------- VoiceNet: (214) 250-3311 Data: -6272 |"Unix doesn't have bugs, InterNet: jfh@rpp386.Dallas.TX.US | Unix is a bug" UucpNet : <backbone>!killer!rpp386!jfh +-- -- author forgotten --
arosen@hawk.ulowell.edu (MFHorn) (12/22/88)
> | nice(-255); /* always win race condition - fixes security bug */ > > Unfortunately, this analysis is incorrect. > The real window > occurs while the mkdir process is blocked on disk I/O Still not right. > Incidentally, the fix proposed by jfh@rpp386 (using dir/./. > as the target of the chown()) doesn't help either. But it's the right idea. A couple years ago, I had to fix this bug in one of our systems. I had source to mkdir.c, but not to the kernel, and was able to successfully close the hole completely. mknod(dirname); /* Irrelevant arguments omitted */ link("."); link(".."); chown(dirname); The real problem is mkdir trusts dirname to be the directory it just created, which is not necessarily the case. Nicing the process only shrinks the window of vunlerability, but it doesn't close it. The proper fix is to change 'chown(dirname);' to 'chown(".");' and add a chdir(dirname); in the right place (with proper error checking). mknod(dirname); link("."); link(".."); chdir(dirname); chown("."); Now, it doesn't matter where the rmdir/ln magic is performed. If it's between chdir and chown, you're too late. If it's between mknod and link, you're too early. If it's between link and chdir, then the chdir fails with ENOTDIR (remember only root can ln directories). Andy Rosen | arosen@hawk.ulowell.edu | "I got this guitar and I ULowell, Box #3031 | ulowell!arosen | learned how to make it Lowell, Ma 01854 | | talk" -Thunder Road RD in '88 - The way it should've been
jfh@rpp386.Dallas.TX.US (The Beach Bum) (12/22/88)
In article <871@husc6.harvard.edu> ddl@husc6.harvard.edu (Dan Lanciani) writes: > Incidentally, the fix proposed by jfh@rpp386 (using dir/./. >as the target of the chown()) doesn't help either. It was a good >try (and happened to be included in the mkdir test mentioned above) >but breaks down since link() itself is not atomic. It is time for this Dan Lanciani person to shut up, or produce proof that these bug fixes do not work. I challenge him to produce a test which will break the mkdir Doug Davis provided with the patch I suggested. Furthermore, I am willing to let that test pound on my system for a day or more if needed. Failing this, I suggest we all add Mr. Laniciani to our official list of crackpots and throw him in the KILL file. The basis for my patch is that the link() call is PRIVILEGED. Since '.' in the context of the above referenced chown() MUST be a directory, the bad guy would have to be root. If Mr. Lanciani is assuming one may become root to break this program, then all bets are off, since Doug's entire assumption is based on the bad guy not becoming root. The bad guy simply can't create a forged directory structure without first BEING root. -- John F. Haugh II +-Quote of the Week:------------------- VoiceNet: (214) 250-3311 Data: -6272 |"Unix doesn't have bugs, InterNet: jfh@rpp386.Dallas.TX.US | Unix is a bug" UucpNet : <backbone>!killer!rpp386!jfh +-- -- author forgotten --
lmb@vicom.COM (Larry Blair) (12/23/88)
In article <871@husc6.harvard.edu> ddl@husc6.harvard.edu (Dan Lanciani) writes: =In article <379@skep2.ATT.COM>, wcs@skep2.ATT.COM (Bill.Stewart.[ho95c]) writes: =| nice(-255); /* always win race condition - fixes security bug */ =| /* nice(-255) is not very nice, but root has its privileges */ =| /* works with official mkdir and Doug's */ =| = Unfortunately, this analysis is incorrect. The real window =occurs while the mkdir process is blocked on disk I/O to, e.g., look =up elements of the path name of the file to chown(). I don't what version of Unix you run, but I don't know of one that would do ANY disk I/O for the chown, since the mknod would have brought all of the elements into core and, being -255, no one else could have caused the kernel to de-cache the inodes and buffers. I'm also pretty sure that there is no danger in this case of losing the CPU due to timeout. I'm sure that some kernel hacker can tell us whether nice(-255) can be preempted. -- Larry Blair ames!vsi1!lmb lmb@vicom.com
ddl@husc6.harvard.edu (Dan Lanciani) (12/23/88)
In article <10845@swan.ulowell.edu>, arosen@hawk.ulowell.edu (MFHorn) writes: | > | nice(-255); /* always win race condition - fixes security bug */ | > | > Unfortunately, this analysis is incorrect. | > The real window | > occurs while the mkdir process is blocked on disk I/O | | Still not right. Eh? | > Incidentally, the fix proposed by jfh@rpp386 (using dir/./. | > as the target of the chown()) doesn't help either. | | But it's the right idea. | | A couple years ago, I had to fix this bug in one of our systems. I had | source to mkdir.c, but not to the kernel, and was able to successfully | close the hole completely. | | | mknod(dirname); /* Irrelevant arguments omitted */ | link("."); | link(".."); | chown(dirname); | | The real problem is mkdir trusts dirname to be the directory it just | created, which is not necessarily the case. Nicing the process only | shrinks the window of vunlerability, but it doesn't close it. Correct. | The proper fix is to change 'chown(dirname);' to 'chown(".");' and | add a chdir(dirname); in the right place (with proper error checking). | | mknod(dirname); | link("."); | link(".."); | chdir(dirname); | chown("."); | | Now, it doesn't matter where the rmdir/ln magic is performed. If it's | between chdir and chown, you're too late. If it's between mknod and | link, you're too early. If it's between link and chdir, then the chdir | fails with ENOTDIR (remember only root can ln directories). But consider that you make *two* switches, one between the lookup of the arguments of the link(dirname, dirname/.) call and one immediately thereafter. (This is what I meant about link itself not being atomic.) This is easier than it sounds because you can sit there switching links between the file you want to chown and the directory. About half the time the link() will fail because it trys to link into your file instead of a directory. The other half the time you will have "." in the newly made directory pointing to the file you want to chown. The subsequent chdir() will succeed about half the time also, for a total success rate of about 1/4. In reality, the granularity of the scheduler makes for a much worse success rate, but it does work. (Yes, I ignored the order of the link() for ".." in your example; it would presumably cut things down to 1/8.) Dan Lanciani ddl@harvard.*
jfh@rpp386.Dallas.TX.US (The Beach Bum) (12/23/88)
In article <1313@vsi1.COM> lmb@vicom.COM (Larry Blair) writes: >I don't what version of Unix you run, but I don't know of one that would >do ANY disk I/O for the chown, since the mknod would have brought all of >the elements into core and, being -255, no one else could have caused the >kernel to de-cache the inodes and buffers. This is an unwarranted assumption, as we shall see later ... If you want to insure the inode will remain cached [ the inode is the only thing you can really get a handle on, paths are very slippery as it were ] you would have to open the directory. The inode would remain resident so long as the directory were open. Or, you could chdir() to the new directory as current directories are also kept in core. Simply praying you have enough inode table entries don't cut it. Inodes are only kept resident for open file, current directories, root directories, pure text files currently under execution or with the sticky bit set, and mounted on directories. [ Plus the one or two others I forgot, like the system accounting file ;-) ] > I'm also pretty sure that there >is no danger in this case of losing the CPU due to timeout. I'm sure that >some kernel hacker can tell us whether nice(-255) can be preempted. There is obviously a VERY severe misunderstanding of the function of nice() under System V. NICE only makes you a preferred process. The actual selection for scheduling is based on your CPU usage, your NICE value, and your priority. Your priority is adjusted after event wakeups and during execution every second or so by the clock interrupt. Your milage may vary. First, some accounting numbers to illustrate the point - START: Thu Dec 22 18:19:08 1988 END: Thu Dec 22 18:22:06 1988 COMMAND START END REAL CPU MEAN NAME USER TTYNAME TIME TIME (SECS) (SECS) SIZE(K) ogetty root ? 18:08:38 18:30:35 1317.12 0.62 369.80 #ogetty root ? 18:15:23 18:27:43 740.16 0.70 363.66 #sh news ? 18:10:00 18:26:22 982.40 0.08 699.50 sh news ? 18:10:00 18:26:22 981.92 0.16 530.50 rnews news ? 18:18:33 18:23:52 319.20 69.48 1433.70 #csh jfh tty02 23:27:53 18:23:20 68126.72 4.92 623.34 csh jfh tty01 18:15:45 18:23:17 452.32 1.10 745.59 #csh root tty01 18:16:25 18:23:06 400.80 1.94 637.69 #mkdir root tty01 18:19:08 18:22:06 177.60 82.08 386.98 . lc jfh tty02 18:21:54 18:21:57 2.94 1.12 1044.84 . #sh news ? 18:20:00 18:20:45 45.12 0.08 529.00 . sh news ? 18:20:01 18:20:45 44.28 0.26 446.30 . uux news ? 18:20:04 18:20:45 40.96 2.08 769.84 . sh news ? 18:20:04 18:20:23 18.72 0.02 727.88 . compress news ? 18:20:04 18:20:23 18.58 0.22 6068.36 . batch news ? 18:20:04 18:20:13 9.12 0.14 463.14 . sh news ? 18:20:10 18:20:12 2.04 0.12 393.83 . expr news ? 18:20:03 18:20:04 0.72 0.04 526.00 All of the commands marked with a '.' in the first column began and ended while mkdir was running. mkdir was running with a NICE of 0, which for System V is as low as it gets. You CAN'T set NICE < 0 under System V [ maybe SVR3? SVR4??? ]. All of the other commands listed were loaded and executing [ except maybe the getty's ] at the same time as mkdir. However, what is most important to notice is that the REAL TIME and the CPU TIME for mkdir were not equal. So long as this is true, a window exists during which another command can enter the job mix and be executed. If you would like to know what arguments I used, try mkdir {A..Z}{A..Z}, that should do the trick [ modulo csh syntax ]. The 'lc' in there should prove interesting as lc had to execute a filename lookup on the 300 or so directories which existed at the time. Clearly if lc can execute 300 stat() calls, some bad guy's program would have little trouble slipping an unlink()/link() pair in there ... Oh well, so much for the nice() mythology. Another UNIX legend down the tubes. Now excuse me as I go run off and rmdir all 676 of those directories I just created ;-( -- John F. Haugh II +-Quote of the Week:------------------- VoiceNet: (214) 250-3311 Data: -6272 |"Unix doesn't have bugs, InterNet: jfh@rpp386.Dallas.TX.US | Unix is a bug" UucpNet : <backbone>!killer!rpp386!jfh +-- -- author forgotten --
wcs@skep2.ATT.COM (Bill.Stewart.[ho95c]) (12/23/88)
I had suggested using nice(-255) to make sure "never" lose the
race condition, with the obvious implication that there's no
guarantee that you really never lose - the suggestion of
mkdiring a large number of files at once to make sure you run
for a long time is one I should have seen. It was meant to be
a safe fix for most of the problem, and does seem to help the
immediate threat.
For what it's worth, I tested the breaking-in
script from the net for about two hours without it losing, vs.
the typical 30 seconds I got with the original mkdir. I've
done less extensive with mkdir foo foo foo foo...
but the nice() gives it enough advantage in the race that
tests of less than a few million iterations aren't very convincing.
Andy Rosen's solution looks right - has anyone found holes in it?
mknod(dirname); link("."); link(".."); chdir(dirname); chown(".");
To make it a bit more general, I assume you would pwd() at the
beginning, and chdir(origdir) before each call to the mkdir routine.
Several people have quibbled about the -255 value - my System V
and 4.1BSD manuals both say that nice() will truncate if the
nice value is out of bounds, so this value makes it truncate
at the least niceness, regardless of system-dependent
differences in what the values mean.
Don writes about my approach:
> = Unfortunately, this analysis is incorrect. The real window
> =occurs while the mkdir process is blocked on disk I/O to, e.g., look
> =up elements of the path name of the file to chown().
In article <1313@vsi1.COM> lmb@vicom.COM (Larry Blair) writes:
> I don't what version of Unix you run, but I don't know of one that would
> do ANY disk I/O for the chown, since the mknod would have brought all of
> the elements into core and, being -255, no one else could have caused the
> kernel to de-cache the inodes and buffers.
Any comments on who's right? I suppose that if you mkdir
enough files and there's a lot of disk thrashing on the
system, it's possible you'll age enough to occasionally
need disk i/o, which means there's some risk of losing the race.
Adding a sync() before each mknod may help.
From: arosen@hawk.ulowell.edu (MFHorn) (Andy Rosen)
> The real problem is mkdir trusts dirname to be the directory it just
> created, which is not necessarily the case. Nicing the process only
> shrinks the window of vunlerability, but it doesn't close it.
Of course, though it makes it much safer, at least for
uniprocessors.
> The proper fix is to change 'chown(dirname);' to 'chown(".");' and
> add a chdir(dirname); in the right place (with proper error checking).
> Now, it doesn't matter where the rmdir/ln magic is performed. If it's
From: ddl@husc6.harvard.edu (Dan Lanciani)
> Incidentally, the fix proposed by jfh@rpp386 (using dir/./.
> as the target of the chown()) doesn't help either. It was a good
> try (and happened to be included in the mkdir test mentioned above)
> but breaks down since link() itself is not atomic.
In article <871@husc6.harvard.edu> ddl@husc6.harvard.edu (Dan Lanciani) writes:
> Incidentally, the fix proposed by jfh@rpp386 (using dir/./.
>as the target of the chown()) doesn't help either. ...
>but breaks down since link() itself is not atomic.
From: John F. Haugh II
> Failing this, I suggest we all add Mr. Laniciani to our official
> list of crackpots and throw him in the KILL file.
Let's not flame here - Don often expresses opinions contrary to the
conventional wisdom, but he knows what he's doing at least as
often as I do :-)
The chdir(dname); chown(".", getuid()) solution looks good -
I'd been looking at variations on
mknod(dname); link dname . ; link parent .. ;
stat the new directory to get permissions
chmod 000 directory
create a subdirectory dname/temp, make . and ..
chown dname/temp/.. /* *must* be a directory */
chmod original-permissions dname
--
# Thanks;
# Bill Stewart, AT&T Bell Labs 2G218 Holmdel NJ 201-949-0705 ho95c.att.com!wcs
#
# News. Don't ask me about News.jfh@rpp386.Dallas.TX.US (The Beach Bum) (12/24/88)
In article <876@husc6.harvard.edu> ddl@husc6.harvard.edu (Dan Lanciani) writes: >| The real problem is mkdir trusts dirname to be the directory it just >| created, which is not necessarily the case. Nicing the process only >| shrinks the window of vunlerability, but it doesn't close it. > > Correct. In the case of the posted patch which I suggested, this is immaterial. If the directory being chown()'d is NOT the directory which was just created, then the person doing the spoofing must have created the bogus directory with some help by becoming root, since only root could have made the bogus directory links. Given THAT piece of information, it SHOULD be obvious that either the patch works, or the bad guy was root already in which case it doesn't matter what the hell happens. Only root can create arbitrary directory structures. Only a clever manipulation of the directory structure could cause mkdir to chown the wrong directory. -- John F. Haugh II +-Quote of the Week:------------------- VoiceNet: (214) 250-3311 Data: -6272 |"Unix doesn't have bugs, InterNet: jfh@rpp386.Dallas.TX.US | Unix is a bug" UucpNet : <backbone>!killer!rpp386!jfh +-- -- author forgotten --