Re: tmpfile security hole

[Jeff Johnston <jjohnstn at redhat dot com>]

Yes, thanks.

-- Jeff J.

Eric Blake wrote:
> 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 */
> -