Re: Symlinks and Cryogenic Sleep

[christos () ZOULAS COM (Christos Zoulas)]

On Jan 4, 12:11pm, babinebell () TRUSTCENTER DE (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