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 --
daveb@geaclib.UUCP (David Collier-Brown) (12/25/88)
From article <10845@swan.ulowell.edu>, by arosen@hawk.ulowell.edu (MFHorn): > 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. Just a sidebar on the presumed safety of Berkeley 4.2: while creating old-fashioned lock files on an Ultrix system, I noted that at least the rename system call was non-atomic (ie, I could observe both the old and new names for a few repetitions of ls -l), and wonder if a similar situation applies to the mkdir operation. --dave (I'll try it on my sun next year (:-)) c-b -- David Collier-Brown. | yunexus!lethe!dave Interleaf Canada Inc. | 1550 Enterprise Rd. | He's so smart he's dumb. Mississauga, Ontario | --Joyce C-B