Re: Symlinks and Cryogenic Sleep

From: Christos Zoulas (christosat_private)
Date: Tue Jan 04 2000 - 14:42:31 PST

  • Next message: David Malone: "Re: Flaw in 3c59x.c or in Kernel?" 

    On Jan 4, 12:11pm, babinebellat_private (Goetz Babin-Ebell) wrote:
-- Subject: Re: Symlinks and Cryogenic Sleep

| At 21:24 03.01.00 +0100, Olaf Kirch wrote:
| >Hi all,
| Hallo Olaf,
|
| >when you're dealing with files in /tmp that are supposed to be re-opened
| >(rather than opened once and then discarded) there's an established
| >way to do it which goes like this:
| >
| >	if (lstat(fname, &stb1) >= 0 && S_ISREG(stb1.st_mode)) {
| >		fd = open(fname, O_RDWR);
| >		if (fd < 0 || fstat(fd, &stb2) < 0
| >		 || ino_or_dev_mismatch(&stb1, &stb2))
| >			raise_big_stink()
| >	} else {
| >		/* do the O_EXCL thing */
| >	}
|
| I did something that way:
|
| FILE *DoOpen(const char *cpFile, long bAppend)
| {
|    FILE *spNew;
|    FILE *spTest;
|    struct stat sStat;
|

Bug 1:
	Now you create a file if it does not exist, so I can
	use your little program as a denial of service attack.
	This is why open(O_RDWR) was used above instead of
	(O_RDWR|O_CREAT).

|    spTest = fopen(cpFile,"a");
|    if (!spTest)
|    {
|       Log("ERR FILE OPEN",cpFile);
|       return NULL;
|    }

Bug 2:
	You've just added a race condition; I could have renamed
	the file between the fopen() and lstat(). This is why the
	example is using fstat()

|    if (lstat(cpFile,&sStat))
|    {

Bug 3:
	Now you forgot to close the file before you returned.

|       Log("ERR STAT",cpFile);
|       return NULL;
|    }
|    if ((sStat.st_mode & S_IFMT) == S_IFLNK)
|    {
|       fclose(spTest);
|       Log("ERR ISLINK",cpFile);
|       return NULL;
|    }
|    if (bAppend)
|       spNew = spTest;
|    else
|    {

Bug 4:
	You've just added another race condition; I could have renamed
	the file between the lstat() and fopen().

|       spNew = freopen(cpFile,"w",spTest);
|       fclose(spTest);
|    }
|    if (!spNew)
|    {
|       Log("ERR FILE OPEN",cpFile);
|       return NULL;
|    }
|    return spFile;
| }

Moral of the story: Stick with the code in the example and don't roll
your own; use fdopen() if you need stdio afterwards.

I hope that you are not writing any security critical code...

christos

    This archive was generated by hypermail 2b30 : Fri Apr 13 2001 - 15:26:33 PDT