On Thu, 25 Apr 2002, James Sneeringer wrote:
> On Thu, Apr 25, 2002 at 09:36:37AM -0400, Drew Weaver wrote:
> > Im getting this error when I compile on RH7.2, I've compiled
> > qpopper on like 3 different OS, one was an earlier version of RH
> > (6.2) and I've never seen this before, ideas?
> >
> > ../common/libcommon.a(maillock.o): In function `Qmaillock':
> > /root/qpopper4.0.4/common/maillock.c:278: the use of `tempnam' is
> > dangerous, better use `mkstemp'
>
> Newer versions of gcc give this warning message. It's actually been
> in the tempnam(3) manpage on Linux for some time:
>
> BUGS
> The precise meaning of `appropriate' is undefined; it is
> unspecified how accessibility of a directory is deter-
> mined. Never use this function. Use mkstemp(3) instead.
>
> Unfortunately, mkstemp(3) has different semantics, so it isn't a
> drop-in replacement. Furthermore, not all systems have mkstemp(3),
> and tempnam(3) is still better than mktemp(3).
Well, the problem with tempnam(3) is that it doesn't actually create
the temporary filename it chooses. Thus, using tempnam(3) exposes a
race condition: an attacker could try to predict the filename that
tempnam(3) will return, and create that file before the caller does.
The mktemp(3) function suffers from the same problem. However,
mktemp(3) is worse from a security standpoint, because some
implementations of mktemp(3) choose filenames which are pathetically
easy to predict (e.g., the process id plus a letter).
The mkstemp(3) function is best; it uses an algorithm to generate the
temporary filename which is difficult to predict, and it goes ahead
and opens that file (returning the file descriptor to the caller).
That being said, I believe the risk of qpopper using tempnam(3) is
minimal, for the following reasons:
1. The tempnam(3) function is only called if the no-atomic-open
option is set. The intent of the no-atomic-open option is to
try to get qpopper to work if the mail spool is located on an
NFS server. Because putting the mail spool on an NFS server
is a really stupid thing, it's not very common, and thus I
would expect very few sites to have set the no-atomic-open
option.
2. Qpopper is creating a temporary file in the mail spool
directory, which on many (most?) systems is not
world-writable. This makes it much more difficult for an
attacker to exploit the race condition; he would have to get
some program with the privileges to create files in the mail
spool directory to create the temporary filename on the
attacker's behalf.
3. Qpopper doesn't blindly assume that the filename returned by
tempnam(3) doesn't exist; rather, qpopper uses O_CREAT|O_EXCL
to open the file. If the filename exists, qpopper logs a
debugging message (if debugging is enabled), and calls
tempnam(3) again. Qpopper will keep calling tempnam(3) until
it gets a filename that it can successfully open with
O_CREAT|O_EXCL.
Although I think gcc is correct to generically warn about the use of
tempnam(3), in qpopper's case, the risk of using it is very low.
Nonetheless, it wouldn't be difficult at all to teach qpopper to test
for mkstemp(3) and use it instead of tempnam(3). You'd only have to
touch configure.in, config.in, and common/maillock.c. (I will
probably do this eventually, and submit a patch, but at the moment,
since I don't set the no-atomic-open at our site, it's not a
priority...)
--
James Ralston, Information Technology
Software Engineering Institute
Carnegie Mellon University, Pittsburgh, PA, USA
