Subject: Re: Patch for snprintf/vsnprintf bugs.
To: Krister Walfridsson <cato@df.lth.se>
From: Jaromir Dolecek <jdolecek@netbsd.org>
List: tech-userlevel
Date: 01/05/2003 11:06:40
Looks fine.

Jaromir

Krister Walfridsson wrote:
> There are two rather serious (IMHO) bugs in snprintf/vsnprintf.
> 
> 1. snprintf(foo, 0. "XXX") is guaranteed not to write in foo by the
>    standard (ISO/IEC 9899 7.19.6.5) but our implementation handles this
>    as if the buffer has a size of (size_t)-1.
> 
> 2. snprintf(NULL, 0, "XXX") leaks memory since cantwrite() allocates
>    memory if _bf._base == NULL, and this buffer is never freed
>    (PR 16483).
> 
> I have solved this by adding a dummy buffer for the cases when the
> functions don't get a buffer it may write in.
> 
> OK to commit?
> 
>    /Krister
> 
> 
> 
> Index: stdio/snprintf.c
> ===================================================================
> RCS file: /cvsroot/src/lib/libc/stdio/snprintf.c,v
> retrieving revision 1.16
> diff -u -r1.16 snprintf.c
> --- stdio/snprintf.c	2002/05/26 14:44:00	1.16
> +++ stdio/snprintf.c	2003/01/04 20:36:47
> @@ -65,6 +65,7 @@
>  	va_list ap;
>  	FILE f;
>  	struct __sfileext fext;
> +	unsigned char dummy[1];
> 
>  	_DIAGASSERT(n == 0 || str != NULL);
>  	_DIAGASSERT(fmt != NULL);
> @@ -77,8 +78,16 @@
>  	_FILEEXT_SETUP(&f, &fext);
>  	f._file = -1;
>  	f._flags = __SWR | __SSTR;
> -	f._bf._base = f._p = (unsigned char *)str;
> -	f._bf._size = f._w = n - 1;
> +	if (n == 0)
> +	{
> +		f._bf._base = f._p = dummy;
> +		f._bf._size = f._w = 0;
> +	}
> +	else
> +	{
> +		f._bf._base = f._p = (unsigned char *)str;
> +		f._bf._size = f._w = n - 1;
> +	}
>  	ret = vfprintf(&f, fmt, ap);
>  	*f._p = 0;
>  	va_end(ap);
> Index: stdio/vsnprintf.c
> ===================================================================
> RCS file: /cvsroot/src/lib/libc/stdio/vsnprintf.c,v
> retrieving revision 1.16
> diff -u -r1.16 vsnprintf.c
> --- stdio/vsnprintf.c	2001/12/07 11:47:45	1.16
> +++ stdio/vsnprintf.c	2003/01/04 20:36:47
> @@ -66,6 +66,7 @@
>  	int ret;
>  	FILE f;
>  	struct __sfileext fext;
> +	unsigned char dummy[1];
> 
>  	_DIAGASSERT(n == 0 || str != NULL);
>  	_DIAGASSERT(fmt != NULL);
> @@ -78,8 +79,16 @@
>  	_FILEEXT_SETUP(&f, &fext);
>  	f._file = -1;
>  	f._flags = __SWR | __SSTR;
> -	f._bf._base = f._p = (unsigned char *)str;
> -	f._bf._size = f._w = n - 1;
> +	if (n == 0)
> +	{
> +		f._bf._base = f._p = dummy;
> +		f._bf._size = f._w = 0;
> +	}
> +	else
> +	{
> +		f._bf._base = f._p = (unsigned char *)str;
> +		f._bf._size = f._w = n - 1;
> +	}
>  	ret = vfprintf(&f, fmt, ap);
>  	*f._p = 0;
>  	return (ret);
> 


-- 
Jaromir Dolecek <jdolecek@NetBSD.org>            http://www.NetBSD.org/
-=- We should be mindful of the potential goal, but as the tantric    -=-
-=- Buddhist masters say, ``You may notice during meditation that you -=-
-=- sometimes levitate or glow.   Do not let this distract you.''     -=-