gl_{,SN}PRINTF_DIRECTIVE_N wrongly fail on Ubuntu 18.04

gl_{,SN}PRINTF_DIRECTIVE_N wrongly fail on Ubuntu 18.04

[Adrian Bunk]

rpl_fprintf is wrongly being used on Ubuntu 18.04 due to:

$ cat test.c
/* gl_PRINTF_DIRECTIVE_N */
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
static char fmtstring[10];
static char buf[100];
int main ()
{
  int count = -1;
  /* Copy the format string.  Some systems (glibc with _FORTIFY_SOURCE=2)
     support %n in format strings in read-only memory but not in writable
     memory.  */
  strcpy (fmtstring, "%d %n");
  if (sprintf (buf, fmtstring, 123, &count, 33, 44, 55) < 0
      || strcmp (buf, "123 ") != 0
      || count != 4)
    return 1;
  return 0;
}
$ gcc -O2 test.c -o test && ./test
*** %n in writable segment detected ***
Aborted
$ 

cu
Adrian

Re: gl_{,SN}PRINTF_DIRECTIVE_N wrongly fail on Ubuntu 18.04

[Bruno Haible]

Hi Adrian,

> rpl_fprintf is wrongly being used on Ubuntu 18.04 due to:
> 
> $ cat test.c
> /* gl_PRINTF_DIRECTIVE_N */
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> static char fmtstring[10];
> static char buf[100];
> int main ()
> {
>   int count = -1;
>   /* Copy the format string.  Some systems (glibc with _FORTIFY_SOURCE=2)
>      support %n in format strings in read-only memory but not in writable
>      memory.  */
>   strcpy (fmtstring, "%d %n");
>   if (sprintf (buf, fmtstring, 123, &count, 33, 44, 55) < 0
>       || strcmp (buf, "123 ") != 0
>       || count != 4)
>     return 1;
>   return 0;
> }
> $ gcc -O2 test.c -o test && ./test
> *** %n in writable segment detected ***
> Aborted
> $ 

gnulib works as designed. gnulib is designed to override system function so as
to make them POSIX compliant. POSIX [1] specifies that support for %n in the
*printf functions is mandatory. As you have shown with the test program, glibc
does not provide %n support when the CFLAGS contain _FORTIFY_SOURCE=2; gnulib
rectifies that.

Bruno

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/printf.html

Re: gl_{,SN}PRINTF_DIRECTIVE_N wrongly fail on Ubuntu 18.04

[Adrian Bunk]

On Sun, Mar 08, 2020 at 06:59:54PM +0100, Bruno Haible wrote:
> Hi Adrian,

Hi Bruno,

> gnulib works as designed. gnulib is designed to override system function so as
> to make them POSIX compliant. POSIX [1] specifies that support for %n in the
> *printf functions is mandatory. As you have shown with the test program, glibc
> does not provide %n support when the CFLAGS contain _FORTIFY_SOURCE=2; gnulib
> rectifies that.

it is unfortunate when GNU software like bison or gzip uses the
gnulib replacement for most glibc users.

In that case there are one documentation deficit and two gnulib bugs:

    * This function does not support the 'n' directive on some platforms:
-     MSVC 14.
+     MSVC 14, glibc with _FORTIFY_SOURCE > 0 (default in some
+     Linux distributions).


The cross compile cases are then bogus for glibc:

printf:
 *)               gl_cv_func_printf_directive_n="guessing yes";;

snprintf:
 *-gnu* | gnu*)        gl_cv_func_snprintf_directive_n="guessing yes";;


> Bruno
>...

cu
Adrian

Re: gl_{,SN}PRINTF_DIRECTIVE_N wrongly fail on Ubuntu 18.04

[Bruno Haible]

Hi Adrian,

> In that case there are one documentation deficit and two gnulib bugs:
> 
>     * This function does not support the 'n' directive on some platforms:
> -     MSVC 14.
> +     MSVC 14, glibc with _FORTIFY_SOURCE > 0 (default in some
> +     Linux distributions).
> 
> 
> The cross compile cases are then bogus for glibc:
> 
> printf:
>  *)               gl_cv_func_printf_directive_n="guessing yes";;
> 
> snprintf:
>  *-gnu* | gnu*)        gl_cv_func_snprintf_directive_n="guessing yes";;

Fixed through the patch below.

> it is unfortunate when GNU software like bison or gzip uses the
> gnulib replacement for most glibc users.

You mean, because of code size of the executable? In this case it would
make sense for gnulib to have "nearly POSIX" compliant variants of these
functions; this would remove the need for the gnulib *printf* code in
many cases.

Or because of security considerations (%n being used to construct "gadgets"
in malware)? In this case, it would be good to take this up with the ISO C
and POSIX standardization groups. Gnulib will adapt, as new versions of the
standards are released.


2020-03-08  Bruno Haible  <bruno@clisp.org>

	*printf-posix: Document why it's overridden on some glibc systems.
	Reported by Adrian Bunk <bunk@stusta.de> in
	<https://lists.gnu.org/archive/html/bug-gnulib/2020-03/msg00011.html>.
	* doc/posix-functions/*printf.texi: Document the problem with the %n
	directive on some glibc systems.
	* m4/printf.m4 (gl_PRINTF_DIRECTIVE_N, gl_SNPRINTF_DIRECTIVE_N): Adjust
	the cross-compilation guesses accordingly.

diff --git a/doc/posix-functions/fprintf.texi b/doc/posix-functions/fprintf.texi
index a596bcb..f555806 100644
--- a/doc/posix-functions/fprintf.texi
+++ b/doc/posix-functions/fprintf.texi
@@ -30,6 +30,7 @@ NetBSD 3.0, AIX 5.1, HP-UX 11.23, IRIX 6.5, Solaris 9,
 Cygwin 1.5.x, mingw, MSVC 14.
 @item
 This function does not support the @samp{n} directive on some platforms:
+glibc when used with @code{_FORTIFY_SOURCE >= 2} (set by default on Ubuntu),
 MSVC 14.
 @item
 This function does not support the @samp{ls} directive on some platforms:
diff --git a/doc/posix-functions/printf.texi b/doc/posix-functions/printf.texi
index 3f00531..1216656 100644
--- a/doc/posix-functions/printf.texi
+++ b/doc/posix-functions/printf.texi
@@ -30,6 +30,7 @@ NetBSD 3.0, AIX 5.1, HP-UX 11.23, IRIX 6.5, Solaris 9,
 Cygwin 1.5.x, mingw, MSVC 14.
 @item
 This function does not support the @samp{n} directive on some platforms:
+glibc when used with @code{_FORTIFY_SOURCE >= 2} (set by default on Ubuntu),
 MSVC 14.
 @item
 This function does not support the @samp{ls} directive on some platforms:
diff --git a/doc/posix-functions/snprintf.texi b/doc/posix-functions/snprintf.texi
index 477931f..6724043 100644
--- a/doc/posix-functions/snprintf.texi
+++ b/doc/posix-functions/snprintf.texi
@@ -45,6 +45,7 @@ NetBSD 3.0, AIX 5.1, HP-UX 11.23, IRIX 6.5, Solaris 9,
 Cygwin 1.5.x, mingw, MSVC 14.
 @item
 This function does not support the @samp{n} directive on some platforms:
+glibc when used with @code{_FORTIFY_SOURCE >= 2} (set by default on Ubuntu),
 MSVC 14.
 @item
 This function does not support the @samp{ls} directive on some platforms:
diff --git a/doc/posix-functions/sprintf.texi b/doc/posix-functions/sprintf.texi
index c43dd15..a880899 100644
--- a/doc/posix-functions/sprintf.texi
+++ b/doc/posix-functions/sprintf.texi
@@ -30,6 +30,7 @@ NetBSD 3.0, AIX 5.1, HP-UX 11.23, IRIX 6.5, Solaris 9,
 Cygwin 1.5.x, mingw, MSVC 14.
 @item
 This function does not support the @samp{n} directive on some platforms:
+glibc when used with @code{_FORTIFY_SOURCE >= 2} (set by default on Ubuntu),
 MSVC 14.
 @item
 This function does not support the @samp{ls} directive on some platforms:
diff --git a/doc/posix-functions/swprintf.texi b/doc/posix-functions/swprintf.texi
index b5aca13..4d849ae 100644
--- a/doc/posix-functions/swprintf.texi
+++ b/doc/posix-functions/swprintf.texi
@@ -27,6 +27,7 @@ This function is only defined as a macro on some platforms:
 MSVC 14.
 @item
 This function does not support the @samp{n} directive on some platforms:
+glibc when used with @code{_FORTIFY_SOURCE >= 2} (set by default on Ubuntu),
 MSVC 14.
 @item
 On Windows and 32-bit AIX platforms, @code{wchar_t} is a 16-bit type and therefore cannot
diff --git a/doc/posix-functions/vfprintf.texi b/doc/posix-functions/vfprintf.texi
index 0e7ecba..a87219e 100644
--- a/doc/posix-functions/vfprintf.texi
+++ b/doc/posix-functions/vfprintf.texi
@@ -30,6 +30,7 @@ NetBSD 3.0, AIX 5.1, HP-UX 11.23, IRIX 6.5, Solaris 9,
 Cygwin 1.5.x, mingw, MSVC 14.
 @item
 This function does not support the @samp{n} directive on some platforms:
+glibc when used with @code{_FORTIFY_SOURCE >= 2} (set by default on Ubuntu),
 MSVC 14.
 @item
 This function does not support the @samp{ls} directive on some platforms:
diff --git a/doc/posix-functions/vprintf.texi b/doc/posix-functions/vprintf.texi
index 09d9e2b..0cb573c 100644
--- a/doc/posix-functions/vprintf.texi
+++ b/doc/posix-functions/vprintf.texi
@@ -30,6 +30,7 @@ NetBSD 3.0, AIX 5.1, HP-UX 11.23, IRIX 6.5, Solaris 9,
 Cygwin 1.5.x, mingw, MSVC 14.
 @item
 This function does not support the @samp{n} directive on some platforms:
+glibc when used with @code{_FORTIFY_SOURCE >= 2} (set by default on Ubuntu),
 MSVC 14.
 @item
 This function does not support the @samp{ls} directive on some platforms:
diff --git a/doc/posix-functions/vsnprintf.texi b/doc/posix-functions/vsnprintf.texi
index de5258d..8f2f410 100644
--- a/doc/posix-functions/vsnprintf.texi
+++ b/doc/posix-functions/vsnprintf.texi
@@ -75,6 +75,7 @@ This function does not truncate the result as specified in C99 on some platforms
 mingw, MSVC 14.
 @item
 This function does not fully support the @samp{n} directive on some platforms:
+glibc when used with @code{_FORTIFY_SOURCE >= 2} (set by default on Ubuntu),
 HP-UX 11, mingw, MSVC 14.
 @item
 This function overwrites memory even when a zero size argument is passed on some
diff --git a/doc/posix-functions/vsprintf.texi b/doc/posix-functions/vsprintf.texi
index 71da5cc..4e264c7 100644
--- a/doc/posix-functions/vsprintf.texi
+++ b/doc/posix-functions/vsprintf.texi
@@ -30,6 +30,7 @@ NetBSD 3.0, AIX 5.1, HP-UX 11.23, IRIX 6.5, Solaris 9,
 Cygwin 1.5.x, mingw, MSVC 14.
 @item
 This function does not support the @samp{n} directive on some platforms:
+glibc when used with @code{_FORTIFY_SOURCE >= 2} (set by default on Ubuntu),
 MSVC 14.
 @item
 This function does not support the @samp{ls} directive on some platforms:
diff --git a/m4/printf.m4 b/m4/printf.m4
index 9df2153..54a2d71 100644
--- a/m4/printf.m4
+++ b/m4/printf.m4
@@ -1,4 +1,4 @@
-# printf.m4 serial 62
+# printf.m4 serial 63
 dnl Copyright (C) 2003, 2007-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,
@@ -711,6 +711,16 @@ int main ()
         [gl_cv_func_printf_directive_n=yes],
         [gl_cv_func_printf_directive_n=no],
         [case "$host_os" in
+                            # Guess no on glibc when _FORTIFY_SOURCE >= 2.
+           *-gnu* | gnu*)   AC_COMPILE_IFELSE(
+                              [AC_LANG_SOURCE(
+                                 [[#if _FORTIFY_SOURCE >= 2
+                                    error fail
+                                   #endif
+                                 ]])],
+                              [gl_cv_func_printf_directive_n="guessing yes"],
+                              [gl_cv_func_printf_directive_n="guessing no"])
+                            ;;
                             # Guess no on Android.
            linux*-android*) gl_cv_func_printf_directive_n="guessing no";;
                             # Guess no on native Windows.
@@ -1414,8 +1424,16 @@ int main ()
         [
 changequote(,)dnl
          case "$host_os" in
-                                 # Guess yes on glibc systems.
-           *-gnu* | gnu*)        gl_cv_func_snprintf_directive_n="guessing yes";;
+                                 # Guess no on glibc when _FORTIFY_SOURCE >= 2.
+           *-gnu* | gnu*)        AC_COMPILE_IFELSE(
+                                   [AC_LANG_SOURCE(
+                                      [[#if _FORTIFY_SOURCE >= 2
+                                         error fail
+                                        #endif
+                                      ]])],
+                                   [gl_cv_func_snprintf_directive_n="guessing yes"],
+                                   [gl_cv_func_snprintf_directive_n="guessing no"])
+                                 ;;
                                  # Guess yes on musl systems.
            *-musl*)              gl_cv_func_snprintf_directive_n="guessing yes";;
                                  # Guess yes on FreeBSD >= 5.

Re: gl_{,SN}PRINTF_DIRECTIVE_N wrongly fail on Ubuntu 18.04

[Adrian Bunk]

On Sun, Mar 08, 2020 at 11:04:40PM +0100, Bruno Haible wrote:
> Hi Adrian,

Hi Bruno,

>...
> it would
> make sense for gnulib to have "nearly POSIX" compliant variants of these
> functions; this would remove the need for the gnulib *printf* code in
> many cases.

this sounds like a good idea.

Looking at git history the usage in bison this is due to
  https://lists.gnu.org/archive/html/bug-gnulib/2009-10/msg00143.html

gzip is due to
  http://git.savannah.gnu.org/cgit/gzip.git/commit/?id=bc4b1e68a8da345c6027dd034592f66808a6f254

So -DGNULIB_POSIXCHECK recommending a "nearly POSIX" compliant variant
could avoid replacing the glibc code with the gnulib code.

>...
> Or because of security considerations (%n being used to construct "gadgets"
> in malware)?

This, and distributions using several copies of the gnulib code instead 
of the fortified and security supported code in glibc.

>...
> --- a/doc/posix-functions/fprintf.texi
> +++ b/doc/posix-functions/fprintf.texi
> @@ -30,6 +30,7 @@ NetBSD 3.0, AIX 5.1, HP-UX 11.23, IRIX 6.5, Solaris 9,
>  Cygwin 1.5.x, mingw, MSVC 14.
>  @item
>  This function does not support the @samp{n} directive on some platforms:
> +glibc when used with @code{_FORTIFY_SOURCE >= 2}

_FORTIFY_SOURCE > 0, the glibc code is under
      if ((mode_flags & PRINTF_FORTIFY) != 0)                                 

> (set by default on Ubuntu),
>...

Ubuntu has enabled it by the default in gcc, which means you get it 
by default not only for package building.

Debian/Gentoo/Fedora/openSUSE (including RHEL/SLES) all add
_FORTIFY_SOURCE by default at least for building packages,
it might be hard to find a distribution that is not doing
compile time hardening in the year 2020.

cu
Adrian

Re: gl_{,SN}PRINTF_DIRECTIVE_N wrongly fail on Ubuntu 18.04

[Bruno Haible]

Hi Adrian,

> >...
> > it would
> > make sense for gnulib to have "nearly POSIX" compliant variants of these
> > functions; this would remove the need for the gnulib *printf* code in
> > many cases.
> 
> this sounds like a good idea.

Paul, Eric, what's your opinion on this?

> > --- a/doc/posix-functions/fprintf.texi
> > +++ b/doc/posix-functions/fprintf.texi
> > @@ -30,6 +30,7 @@ NetBSD 3.0, AIX 5.1, HP-UX 11.23, IRIX 6.5, Solaris 9,
> >  Cygwin 1.5.x, mingw, MSVC 14.
> >  @item
> >  This function does not support the @samp{n} directive on some platforms:
> > +glibc when used with @code{_FORTIFY_SOURCE >= 2}
> 
> _FORTIFY_SOURCE > 0, the glibc code is under
>       if ((mode_flags & PRINTF_FORTIFY) != 0)                                 

Sorry, but I verified on 3 systems (Ubuntu, Debian, Fedora) that
-D_FORTIFY_SOURCE=1 is like -D_FORTIFY_SOURCE=0, when it comes to this
test program.

> > (set by default on Ubuntu),
> >...
> 
> Ubuntu has enabled it by the default in gcc, which means you get it 
> by default not only for package building.
> 
> Debian/Gentoo/Fedora/openSUSE (including RHEL/SLES) all add
> _FORTIFY_SOURCE by default at least for building packages

The documentation we write is for the users of the plain 'gcc' program
(or vendor compilers). We can't consider the packaging systems of various
distros, as this is not something the user sees when compiling programs
as documented in the INSTALL file.

If you are among the packagers for a distro, you'll need to have knowledge
about the packaging systems and combine it with the knowledge you get from
the gnulib documentation.

Bruno

Re: gl_{,SN}PRINTF_DIRECTIVE_N wrongly fail on Ubuntu 18.04

[Paul Eggert]

On 3/8/20 5:11 PM, Bruno Haible wrote:
>>> it would
>>> make sense for gnulib to have "nearly POSIX" compliant variants of these
>>> functions; this would remove the need for the gnulib*printf*  code in
>>> many cases.
>> this sounds like a good idea.
> Paul, Eric, what's your opinion on this?

Might be helpful for programs that don't want to use %n.

Perhaps also there should be some way to tell glibc "Hey, I want to use %n even 
though I want the other fortification features." That'd be an improvement over 
what we have now. For what it's worth, GNU Emacs (which I happen to be working 
on right now) uses %n.

Re: gl_{,SN}PRINTF_DIRECTIVE_N wrongly fail on Ubuntu 18.04

[Bruno Haible]

I wrote:
> 2020-03-08  Bruno Haible  <bruno@clisp.org>
> 
> 	*printf-posix: Document why it's overridden on some glibc systems.
> 	Reported by Adrian Bunk <bunk@stusta.de> in
> 	<https://lists.gnu.org/archive/html/bug-gnulib/2020-03/msg00011.html>.
> 	* doc/posix-functions/*printf.texi: Document the problem with the %n
> 	directive on some glibc systems.
> 	* m4/printf.m4 (gl_PRINTF_DIRECTIVE_N, gl_SNPRINTF_DIRECTIVE_N): Adjust
> 	the cross-compilation guesses accordingly.

Oops, this patch had a mistake: It produced an error

executing aclocal -I glm4
/usr/bin/m4:configure.ac:4472: recursion limit of 1024 exceeded, use -L<N> to change it
autom4te: /usr/bin/m4 failed with exit status: 1
aclocal: error: echo failed with exit status: 1

Caught by the continuous integration - thanks Tim for setting it up!


2020-03-16  Bruno Haible  <bruno@clisp.org>

	*printf-posix: Fix m4 error (regression from 2020-03-08).
	* m4/printf.m4 (gl_SNPRINTF_DIRECTIVE_N): Enable interpretation of
	brackets in AC_COMPILE_IFELSE invocation.

diff --git a/m4/printf.m4 b/m4/printf.m4
index 54a2d71..df473d8 100644
--- a/m4/printf.m4
+++ b/m4/printf.m4
@@ -1,4 +1,4 @@
-# printf.m4 serial 63
+# printf.m4 serial 64
 dnl Copyright (C) 2003, 2007-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,
@@ -1422,7 +1422,6 @@ int main ()
         [gl_cv_func_snprintf_directive_n=yes],
         [gl_cv_func_snprintf_directive_n=no],
         [
-changequote(,)dnl
          case "$host_os" in
                                  # Guess no on glibc when _FORTIFY_SOURCE >= 2.
            *-gnu* | gnu*)        AC_COMPILE_IFELSE(
@@ -1434,6 +1433,7 @@ changequote(,)dnl
                                    [gl_cv_func_snprintf_directive_n="guessing yes"],
                                    [gl_cv_func_snprintf_directive_n="guessing no"])
                                  ;;
+changequote(,)dnl
                                  # Guess yes on musl systems.
            *-musl*)              gl_cv_func_snprintf_directive_n="guessing yes";;
                                  # Guess yes on FreeBSD >= 5.
@@ -1466,8 +1466,8 @@ changequote(,)dnl
            mingw*)               gl_cv_func_snprintf_directive_n="guessing no";;
                                  # If we don't know, obey --enable-cross-guesses.
            *)                    gl_cv_func_snprintf_directive_n="$gl_cross_guess_normal";;
-         esac
 changequote([,])dnl
+         esac
         ])
     ])
 ])