mkdir() and security hole ***** ONE-LINE FIX !! ****

[Bill.Stewart.[ho95c]]

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 at vsi1.COM> lmb at 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 at 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 at husc6.harvard.edu (Dan Lanciani)
> 	Incidentally, the fix proposed by jfh at 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 at husc6.harvard.edu> ddl at husc6.harvard.edu (Dan Lanciani) writes:
>	Incidentally, the fix proposed by jfh at 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.