That patch is an EXTREMELY BAD IDEA.

It will prevent the c-client library from building.

There is no such function as mkstmp().  You may be thinking of mkstemp().

But even if you use mkstemp(), this patch will prevent the c-client library from building on some platforms.

On most systems, the tmpnam() won't ever be executed, except on old systems which don't have /dev/urandom (and won't have mkstemp() either).

Nevertheless, EVEN IF tmpnam() is executed, there is NO security problem. The issue that the compiler warning identifies DOES NOT occur in the way that tmpnam() is used in this code. It is a FALSE ALARM.

#ifdef is another BAD idea. Unless you have the #ifdef perfectly right for all possible combinations of systems, there will be systems on which you will either
 [1] break the build
 [2] create a REAL, critical security problem (and I do mean critical)

All because of a compiler warning that you have been told in the FAQ is a FALSE alarm.

I understand your good intentions; but sometimes the way to hell is paved with good intentions. This is one of those cases.

The people who put in that compiler warning also had good intentions. That doesn't alter the fact that there are cases where their good intentions are wrong-headed.

On Tue, 14 Jul 2009, David Sommerseth wrote:

The compiler complained about usage of the unsafe tmpnam() function  in
ssl_unix.c.  This patch changes this to make use of the safe mkstmp().

In the FAQ I see that it is claimed that this part of the code is not
executed on Linux, in particular.  If this is the case, I am surprised
this part of the code is not in a removed in a #ifdef block.

The problem gets even a bit more complicated when other programs
linking in the the library statically also comes with the same compiler
warnings.

The code is, from a code perspective, available in the library in binary
form.  In this perspective, it might be a possibility that this code is
executed by some software packages.

This was also the only place in the code where I could find tmpnam()
used in the c-client-2007e library.


kind regards,

David Sommerseth



--- a/src/osdep/unix/ssl_unix.c 2008-06-04 20:18:34.000000000 +0200
+++ b/src/osdep/unix/ssl_unix.c 2009-06-02 13:19:00.000000000 +0200
@@ -98,7 +98,9 @@
    struct stat sbuf;
                               /* if system doesn't have /dev/urandom */
    if (stat ("/dev/urandom",&sbuf)) {
-      while ((fd = open (tmpnam (tmp),O_WRONLY|O_CREAT|O_EXCL,0600)) < 0)
+      strncpy (tmp, "tmpXXXXXX\0", MAILTMPLEN-1);
+      mkstmp (tmp);
+      while ((fd = open (tmp, O_WRONLY|O_CREAT|O_EXCL,0600)) < 0)
       sleep (1);
      unlink (tmp);            /* don't need the file */
      fstat (fd,&sbuf);                /* get information about the file */


_______________________________________________
Imap-uw mailing list
Imap-uw@u.washington.edu
http://mailman2.u.washington.edu/mailman/listinfo/imap-uw


-- Mark --

http://panda.com/mrc
Science does not emerge from voting, party politics, or public debate.
Si vis pacem, para bellum.
_______________________________________________
Imap-uw mailing list
Imap-uw@u.washington.edu
http://mailman2.u.washington.edu/mailman/listinfo/imap-uw

Reply via email to