[PATCH] mktemp: don't use mktemp() function

[Rich Felker]

On Thu, Dec 11, 2014 at 06:01:57PM +0100, Bartosz Golaszewski wrote:
> The linker emits this warning:
> 
>   warning: the use of `mktemp' is dangerous, better use `mkstemp' or `mkdtemp'
> 
> Fix it by using mkstemp() instead of mktemp().
> 
> function                                             old     new   delta
> mktemp_main                                          214     233     +19
> ------------------------------------------------------------------------------
> (add/remove: 0/0 grow/shrink: 1/0 up/down: 19/0)               Total: 19 bytes
> 
> Signed-off-by: Bartosz Golaszewski <bartekgola at gmail.com>
> ---
>  debianutils/mktemp.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/debianutils/mktemp.c b/debianutils/mktemp.c
> index 983d7a2..5ef557a 100644
> --- a/debianutils/mktemp.c
> +++ b/debianutils/mktemp.c
> @@ -59,6 +59,8 @@ int mktemp_main(int argc UNUSED_PARAM, char **argv)
>  	const char *path;
>  	char *chp;
>  	unsigned opts;
> +	int fd;
> +
>  	enum {
>  		OPT_d = 1 << 0,
>  		OPT_q = 1 << 1,
> @@ -93,8 +95,9 @@ int mktemp_main(int argc UNUSED_PARAM, char **argv)
>  		chp = concat_path_file(path, chp);
>  
>  	if (opts & OPT_u) {
> -		chp = mktemp(chp);
> -		if (chp[0] == '\0')
> +		close(fd = mkstemp(chp));
> +		unlink(chp);
> +		if (fd < 0)

How is this an improvement? It increases the code size and performs
unnecessary and potentially harmful filesystem operations. And it's
just covering up the "dangerous" issue rather than fixing it -- using
mkstemp then deleting the file and reusing the name is even MORE
dangerous than using mktemp, since creating the file even momentarily
exposed its name to an attacker. Of course if the code using the
mktemp utility is written correctly, neither is dangerous anyway.

Rich