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

[Dan Lanciani]

In article <10845 at swan.ulowell.edu>, arosen at 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 at 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 at harvard.*