Path: utzoo!utgpu!watmath!clyde!att!osu-cis!tut.cis.ohio-state.edu!mailrus!ames!killer!vector!rpp386!jfh
From: jfh@rpp386.Dallas.TX.US (The Beach Bum)
Newsgroups: news.admin
Subject: Re: mkdir() and security hole   *****FIX****
Summary: And here is part of the fix for the fix ...
Message-ID: <10117@rpp386.Dallas.TX.US>
Date: 19 Dec 88 20:27:37 GMT
References: <9466@merch.TANDY.COM> <851@husc6.harvard.edu> <10115@rpp386.Dallas.TX.US>
Reply-To: jfh@rpp386.Dallas.TX.US (The Beach Bum)
Organization: Big "D" Home for Wayward Hackers
Lines: 134

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 --