[13259] in bugtraq
Re: Symlinks and Cryogenic Sleep
daemon@ATHENA.MIT.EDU (Christos Zoulas)
Wed Jan 5 18:37:35 2000
Message-Id: <200001042242.RAA05778@hrothgar.gw.com>
Date: Tue, 4 Jan 2000 17:42:31 -0500
Reply-To: Christos Zoulas <christos@ZOULAS.COM>
From: Christos Zoulas <christos@ZOULAS.COM>
X-To: Goetz Babin-Ebell <babinebell@TRUSTCENTER.DE>,
BUGTRAQ@SECURITYFOCUS.COM
To: BUGTRAQ@SECURITYFOCUS.COM
In-Reply-To: <3.0.5.32.20000104121119.038a5ea0@mail> from Goetz Babin-Ebell
(Jan 4, 12:11pm)
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
