Hi Jeremie, > The attached patch changes vasnprintf.c to avoid using %n in the general > case, ie when the return value of snprintf is usable. This should help > if more systems decide to make tighten %n usage. There are plans for > that in OpenBSD land.
Thanks for the suggestion. Yes, this is a good idea. (I didn't know about the OpenBSD plans.) > The existing comments in vasnprintf.c mention systems where > gl_SNPRINTF_RETVAL_C99 and gl_SNPRINTF_TRUNCATION_C99 pass. This patch > only considers the usability of the return value of snprintf, as lack > of truncation seems to be a different problem (apparently handled later > in the code). I prefer to limit the shortcut to those platforms where both gl_SNPRINTF_RETVAL_C99 and gl_SNPRINTF_TRUNCATION_C99 pass. These are the conditions I found when considering this code in detail a couple of years ago. Maybe you are right that only one of the conditions is needed; but I don't want to live on the risky edge here. 2020-10-04 Bruno Haible <[email protected]> vasnprintf: Don't use %n on modern, ISO C 99 compliant platforms. Suggested by Jeremie Courreges-Anglas <[email protected]> in <https://lists.gnu.org/archive/html/bug-gnulib/2020-10/msg00010.html>. * m4/vasnprintf.m4 (gl_PREREQ_VASNPRINTF): Define HAVE_SNPRINTF_TRUNCATION_C99. * lib/vasnprintf.c (VASNPRINTF): Don't use %n if HAVE_SNPRINTF_RETVAL_C99 && HAVE_SNPRINTF_TRUNCATION_C99. diff --git a/lib/vasnprintf.c b/lib/vasnprintf.c index 7f75139..b232d14 100644 --- a/lib/vasnprintf.c +++ b/lib/vasnprintf.c @@ -5117,39 +5117,32 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp, #endif *fbp = dp->conversion; #if USE_SNPRINTF -# if ! (((__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 3)) \ - && !defined __UCLIBC__) \ - || (defined __APPLE__ && defined __MACH__) \ - || defined __ANDROID__ \ - || (defined _WIN32 && ! defined __CYGWIN__)) - fbp[1] = '%'; - fbp[2] = 'n'; - fbp[3] = '\0'; -# else - /* On glibc2 systems from glibc >= 2.3 - probably also older - ones - we know that snprintf's return value conforms to - ISO C 99: the tests gl_SNPRINTF_RETVAL_C99 and - gl_SNPRINTF_TRUNCATION_C99 pass. - Therefore we can avoid using %n in this situation. - On glibc2 systems from 2004-10-18 or newer, the use of %n - in format strings in writable memory may crash the program - (if compiled with _FORTIFY_SOURCE=2), so we should avoid it - in this situation. */ - /* On Mac OS X 10.3 or newer, we know that snprintf's return - value conforms to ISO C 99: the tests gl_SNPRINTF_RETVAL_C99 - and gl_SNPRINTF_TRUNCATION_C99 pass. - Therefore we can avoid using %n in this situation. - On Mac OS X 10.13 or newer, the use of %n in format strings - in writable memory by default crashes the program, so we - should avoid it in this situation. */ - /* On Android, we know that snprintf's return value conforms to - ISO C 99: the tests gl_SNPRINTF_RETVAL_C99 and - gl_SNPRINTF_TRUNCATION_C99 pass. - Therefore we can avoid using %n in this situation. - Starting on 2018-03-07, the use of %n in format strings - produces a fatal error (see - <https://android.googlesource.com/platform/bionic/+/41398d03b7e8e0dfb951660ae713e682e9fc0336>), - so we should avoid it. */ +# if ((HAVE_SNPRINTF_RETVAL_C99 && HAVE_SNPRINTF_TRUNCATION_C99) \ + || ((__GLIBC__ > 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ >= 3)) \ + && !defined __UCLIBC__) \ + || (defined __APPLE__ && defined __MACH__) \ + || defined __ANDROID__ \ + || (defined _WIN32 && ! defined __CYGWIN__)) + /* On systems where we know that snprintf's return value + conforms to ISO C 99 (HAVE_SNPRINTF_RETVAL_C99) and that + snprintf always produces NUL-terminated strings + (HAVE_SNPRINTF_TRUNCATION_C99), it is possible to avoid + using %n. And it is desirable to do so, because more and + more platforms no longer support %n, for "security reasons". + In particular, the following platforms: + - On glibc2 systems from 2004-10-18 or newer, the use of + %n in format strings in writable memory may crash the + program (if compiled with _FORTIFY_SOURCE=2). + - On Mac OS X 10.13 or newer, the use of %n in format + strings in writable memory by default crashes the + program. + - On Android, starting on 2018-03-07, the use of %n in + format strings produces a fatal error (see + <https://android.googlesource.com/platform/bionic/+/41398d03b7e8e0dfb951660ae713e682e9fc0336>). + On these platforms, HAVE_SNPRINTF_RETVAL_C99 and + HAVE_SNPRINTF_TRUNCATION_C99 are 1. We have listed them + explicitly in the condition above, in case of cross- + compilation (just to be sure). */ /* On native Windows systems (such as mingw), we can avoid using %n because: - Although the gl_SNPRINTF_TRUNCATION_C99 test fails, @@ -5166,6 +5159,10 @@ VASNPRINTF (DCHAR_T *resultbuf, size_t *lengthp, <https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/set-printf-count-output> So we should avoid %n in this situation. */ fbp[1] = '\0'; +# else /* AIX <= 5.1, HP-UX, IRIX, OSF/1, Solaris <= 9, BeOS */ + fbp[1] = '%'; + fbp[2] = 'n'; + fbp[3] = '\0'; # endif #else fbp[1] = '\0'; diff --git a/m4/vasnprintf.m4 b/m4/vasnprintf.m4 index 4567061..43f070e 100644 --- a/m4/vasnprintf.m4 +++ b/m4/vasnprintf.m4 @@ -1,4 +1,4 @@ -# vasnprintf.m4 serial 37 +# vasnprintf.m4 serial 38 dnl Copyright (C) 2002-2004, 2006-2020 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -74,6 +74,16 @@ AC_DEFUN_ONCE([gl_PREREQ_VASNPRINTF], if the buffer had been large enough.]) ;; esac + dnl Additionally, the use of %n can be eliminated by assuming that snprintf + dnl always produces NUL-terminated strings (no truncation). + AC_REQUIRE([gl_SNPRINTF_TRUNCATION_C99]) + case "$gl_cv_func_snprintf_truncation_c99" in + *yes) + AC_DEFINE([HAVE_SNPRINTF_TRUNCATION_C99], [1], + [Define if the string produced by the snprintf function is always NUL + terminated.]) + ;; + esac ]) # Extra prerequisites of lib/vasnprintf.c for supporting 'long double'
