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.'' -=-