Probably, in this very case it wouldn't harm so much ... [code snipped]
The very worst thing that could happen is that the destination filename
would have a wrong, temporary name, which the user would know so to rename
it by hand... or is it?
There is definitely something worse that could happen. Richard
Heathfield's example covers a case like:
fp = fopen(name, "r");
if (fp != NULL) {
fclose(fp);
/* allow time to pass */
fp = fopen(name, "r");
/* assumption here: fp != NULL */
}
Your code (which I snipped), however, has a case more like this:
fp = fopen(name, "r");
if (fp != NULL) {
fclose(fp);
/* allow time to pass */
...
} else {
---> fp = fopen(name, "w");
...
}
Now, at the line I marked with an arrow "--->", you assume that
since fopen-for-reading returned NULL, the file does not exist (or,
as you noted elsewhere, that it has bizarre permissions, in which
case the user is shooting himself in the foot and there may not be
much we can do about it anyway).
The more serious problem -- and one which occurs in real-world
security holes on these multi-user, multi-process systems that
Richard Heathfield mentioned -- occurs when enough time passes
between the first fopen (with "r") and the second (with "w") so
that the file, which *did not exist* at the time of the first
fopen(), *does* exist at the time of the second.
In particular, consider, on a Unix-like system, what happens if
you "race" your program against another one that does:
/* evilprogram.c */
#include <unistd.h> /* yes, non-ANSI */
#define sensitive_file "/home/user/very_important_data"
#define name "whatever"
int main(void) {
for (;
{
(void) unlink(name);
usleep(1);
(void) symlink(sensitive_file, name);
sleep(1);
}
/*NOTREACHED*/
}
Now, if "evilprogram" runs at the right (wrong?) time while it
races against your program, your code will do:
fp = fopen(name, "r");
and get NULL, because the file named "whatever" does not exist.
Then evilprogram runs and creates the symlink, so that the file
named "whatever" refers to your "very_important_data" file (or,
in the case of the security holes I mentioned earlier, perhaps
something like "/etc/passwd"). Since, in your code, fp==NULL,
you assume that the file named "whatever" *still* does not
exist, and you do:
fp = fopen(name, "w");
and are now overwriting your very important data (or /etc/passwd,
if running as root, e.g.).
(This problem existed before symlinks: since typical systems at
the time had /etc and /tmp on the same file system, one could make
a hard link from /etc/passwd to /tmp/foo and trick a setuid-root
program into writing on /tmp/foo, which was by then actually
/etc/passwd. The solution is to avoid the C library routines,
which are insufficient, and go directly to open() -- and, in the
case of setuid programs, to use the original 4.3BSD setreuid() or
the more modern POSIX-style saved-setuid -- not the original,
broken, System V version, which did not work for the super-user --
to "give up" setuid permissions temporarily. Fundamentally, what
is needed is an atomic "test all permissions and, only if OK, then
do the open" call. This just does not exist in Standard C.)
There really are times one can, and even should, write non-Standard-C
code. The trick is knowing when.
