This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

tmpfile security hole


The current implementation of tmpfile has a data race - there is a window 
between when the name is generated and when the file is opened.  This is a 
particularly bad security hole, since newlib's tmpnam uses a highly-predictable 
algorithm of increments, rather than a more secure algorithm of random 
characters, making it very easy for an attacker to insert a hard link with the 
correct name in between when tmpfile chose the name and opens the file to 
truncate it.  OK to apply this patch?

FYI - the next version of POSIX is marking mktemp, tempnam and tmpnam as 
obsolescent, because of their insecure nature; recommending the use of mkstemp, 
mkdtemp, and tmpfile instead.

2007-05-16  Eric Blake  <ebb9@byu.net>

	Close security hole in tmpfile.
	* libc/stdio/tmpfile.c (_tmpfile_r): Avoid window between filename
	generation and FILE opening.
	* libc/stdio64/tmpfile64.c (_tmpfile64_r): Likewise.

Index: libc/stdio/tmpfile.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/tmpfile.c,v
retrieving revision 1.3
diff -u -p -r1.3 tmpfile.c
--- libc/stdio/tmpfile.c	23 Apr 2004 20:01:55 -0000	1.3
+++ libc/stdio/tmpfile.c	16 May 2007 18:22:02 -0000
@@ -49,6 +49,11 @@ Supporting OS subroutines required: <<cl
 #include <reent.h>
 #include <stdio.h>
 #include <errno.h>
+#include <fcntl.h>
+
+#ifndef O_BINARY
+# define O_BINARY 0
+#endif
 
 FILE *
 _DEFUN(_tmpfile_r, (ptr),
@@ -58,10 +63,19 @@ _DEFUN(_tmpfile_r, (ptr),
   int e;
   char *f;
   char buf[L_tmpnam];
+  int fd;
 
-  if ((f = _tmpnam_r (ptr, buf)) == NULL)
+  do
+    {
+      if ((f = _tmpnam_r (ptr, buf)) == NULL)
+        return NULL;
+      fd = _open_r (ptr, f, O_RDWR | O_CREAT | O_EXCL | O_BINARY,
+                    S_IRUSR | S_IWUSR);
+    }
+  while (fd < 0 && ptr->_errno == EEXIST);
+  if (fd < 0)
     return NULL;
-  fp = _fopen_r (ptr, f, "wb+");
+  fp = _fdopen_r (ptr, fd, "wb+");
   e = ptr->_errno;
   _CAST_VOID _remove_r (ptr, f);
   ptr->_errno = e;
Index: libc/stdio64/tmpfile64.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio64/tmpfile64.c,v
retrieving revision 1.3
diff -u -p -r1.3 tmpfile64.c
--- libc/stdio64/tmpfile64.c	26 Aug 2003 18:09:43 -0000	1.3
+++ libc/stdio64/tmpfile64.c	16 May 2007 18:22:02 -0000
@@ -49,6 +49,11 @@ Supporting OS subroutines required: <<cl
 
 #include <stdio.h>
 #include <errno.h>
+#include <fcntl.h>
+
+#ifndef O_BINARY
+# define O_BINARY 0
+#endif
 
 #ifdef __LARGE64_FILES
 
@@ -60,10 +65,19 @@ _DEFUN (_tmpfile64_r, (ptr),
   int e;
   char *f;
   char buf[L_tmpnam];
+  int fd;
 
-  if ((f = _tmpnam_r (ptr, buf)) == NULL)
+  do
+  {
+     if ((f = _tmpnam_r (ptr, buf)) == NULL)
+        return NULL;
+      fd = _open64_r (ptr, f, O_RDWR | O_CREAT | O_EXCL | O_BINARY,
+                      S_IRUSR | S_IWUSR);
+  }
+  while (fd < 0 && ptr->_errno == EEXIST);
+  if (fd < 0)
     return NULL;
-  fp = _fopen64_r (ptr, (const char *)f, "wb+");
+  fp = _fdopen64_r (ptr, fd, "wb+");
   e = ptr->_errno;
   _CAST_VOID _remove_r (ptr, f);
   ptr->_errno = e;
@@ -81,4 +95,3 @@ _DEFUN_VOID (tmpfile64)
 #endif
 
 #endif /* __LARGE64_FILES */
-



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]