[PATCH] Warn when using mktemp

[Corinna Vinschen]

On Mar 13 14:54, Howland Craig D (Craig) wrote:
> 1)  Looks like the patch has an unintended newline in the middle of the
> warning message.

Indeed.  Sorry about that.  The actual patch won't have that.

> 2)  Brooks makes a good point about the manual needing an accompanying
> addition, should the compile warning be added.

I did that in the below patch which replaces my first one.

> 1)  Why is mktemp.c in the stdio directory when it is a stdlib.h
> function?

It's historically there in BSD.

> 2)  Why is _mktemp_r in stdio.h rather than in stdlib.h?

That looks like a bug to me.  I fixed that in the attached patch.

On Mar 13 14:54, Brooks Moses wrote:
>   Doing a Google
> search on "mktemp mkstemp dangerous" mostly finds mailing-list threads
> of people who are annoyed by the warning.

That's the idea of this warning at least on BSD and on Linux.  And I'd
like to see this warning on Cygwin as well.  Cygwin has a link time
warning for this but on one hand it's using STABS while the new GCC
for Cygwin now creates Dwarf2, and on the other hand this warning is
not even visible when linking an application using mktemp.

> Pedantically, also, that error message should use a semicolon rather
> than a comma, and I think "use `mkstemp' instead" is a bit less
> colloquial as a phrasing than "better use `mkstemp'".

Fine with me.  See patch below.


Thanks,
Corinna


	* libc/include/stdio.h (_mkstemp_r, _mktemp_r): Move declarations
	to stdlib.h.
	* libc/include/stdlib.h (mktemp, _mktemp_r): Warn when using.
	* libc/stdio/mktemp.c: Explain the security risk when using
	mktemp.


Index: libc/include/stdio.h
===================================================================
RCS file: /cvs/src/src/newlib/libc/include/stdio.h,v
retrieving revision 1.54
diff -u -p -r1.54 stdio.h
--- libc/include/stdio.h	6 Mar 2009 09:55:52 -0000	1.54
+++ libc/include/stdio.h	13 Mar 2009 20:48:28 -0000
@@ -411,8 +411,6 @@ int	_EXFUN(_iprintf_r, (struct _reent *,
                _ATTRIBUTE ((__format__ (__printf__, 2, 3))));
 int	_EXFUN(_iscanf_r, (struct _reent *, const char *, ...)
                _ATTRIBUTE ((__format__ (__scanf__, 2, 3))));
-int	_EXFUN(_mkstemp_r, (struct _reent *, char *));
-char *	_EXFUN(_mktemp_r, (struct _reent *, char *));
 FILE *	_EXFUN(_open_memstream_r, (struct _reent *, char **, size_t *));
 void	_EXFUN(_perror_r, (struct _reent *, const char *));
 int	_EXFUN(_printf_r, (struct _reent *, const char *, ...)
Index: libc/include/stdlib.h
===================================================================
RCS file: /cvs/src/src/newlib/libc/include/stdlib.h,v
retrieving revision 1.28
diff -u -p -r1.28 stdlib.h
--- libc/include/stdlib.h	19 Nov 2008 20:55:52 -0000	1.28
+++ libc/include/stdlib.h	13 Mar 2009 20:48:28 -0000
@@ -98,7 +98,9 @@ size_t	_EXFUN(_wcstombs_r,(struct _reent
 #ifndef __STRICT_ANSI__
 #ifndef _REENT_ONLY
 int     _EXFUN(mkstemp,(char *));
-char *  _EXFUN(mktemp,(char *));
+int	_EXFUN(_mkstemp_r, (struct _reent *, char *));
+char *  _EXFUN(mktemp,(char *) _ATTRIBUTE ((warning ("the use of `mktemp' is dangerous; use `mkstemp' instead"))));
+char *	_EXFUN(_mktemp_r, (struct _reent *, char *) _ATTRIBUTE ((warning ("the use of `mktemp' is dangerous; use `mkstemp' instead"))));
 #endif
 #endif
 _VOID	_EXFUN(qsort,(_PTR __base, size_t __nmemb, size_t __size, int(*_compar)(const _PTR, const _PTR)));
Index: libc/stdio/mktemp.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/stdio/mktemp.c,v
retrieving revision 1.9
diff -u -p -r1.9 mktemp.c
--- libc/stdio/mktemp.c	1 May 2007 23:03:36 -0000	1.9
+++ libc/stdio/mktemp.c	13 Mar 2009 20:48:28 -0000
@@ -85,6 +85,13 @@ unless it could not generate an unused f
 provided is not suitable for a filename; in that case, it returns
 <<-1>>.
 
+NOTES
+Never use <<mktemp>>.  The generated filenames are easy to guess and
+there's a race between the test if the file exists and the creation
+of the file.  In combination this makes <<mktemp>> prone to attacks
+and using it is a security risk.  Whenever possible use <<mkstemp>>
+instead.  It doesn't suffer the race condition.
+
 PORTABILITY
 ANSI C does not require either <<mktemp>> or <<mkstemp>>; the System
 V Interface Definition requires <<mktemp>> as of Issue 2.

-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat