Re: cc claims ISO C99 support, but %n printf format specifier calls abort()

[Jeremie Courreges-Anglas]

On Fri, Dec 16 2022, Vincent Lefevre  wrote:
> On 2022-12-15 18:56:15 -0700, Theo de Raadt wrote:
>> There are almost no %n left in the software ecosystem.  If we are able
>> to make this crossing, everyone else is also capable, and eventually
>> will.  Just like with gets().
>
> FYI, this breaks GMP, whose configure script insists on %n being
> available, otherwise GMP uses its own, buggy implementation of
> vsnprintf, which triggers an assertion failure when %a/%A is used
> (and this bug affects MPFR). AFAIK, the GMP developers haven't
> reacted to the bug report sent in October.

https://gmplib.org/list-archives/gmp-bugs/2022-October/005200.html
Between the dubious antique check in acinclude.m4 and the bug in the
*printf reimplementation, this really sound like self-inflicted pain.
The diff below (against gmp-6.2.1) skips the %n test and is enough to
fix the misdetection.  Some tests need to stop using %n too.

Not an OpenBSD bug IMO, better discuss this with the GMP developers.


--- acinclude.m4.orig   Fri Dec 16 20:45:25 2022
+++ acinclude.m4Fri Dec 16 20:51:48 2022
@@ -3664,9 +3664,9 @@ dnl  literal string is enough to provoke the problem, 
 dnl  needed.  There seems to be something weird going on with the optimizer
 dnl  or something, since on the first system adding a second check with
 dnl  "%n", or even just an initialized local variable, makes it work.  In
-dnl  any case, without bothering to get to the bottom of this, the two
-dnl  program runs in the code below end up successfully detecting the
-dnl  problem.
+dnl  any case, without bothering to get to the bottom of this, using %n is
+dnl  frowned upon on several modern systems.  So just assume that vsnprintf
+dnl  works if a simple test works...
 dnl
 dnl  glibc 2.0.x returns either -1 or bufsize-1 for an overflow (both seen,
 dnl  not sure which 2.0.x does which), but still puts the correct null
@@ -3682,7 +3682,6 @@ else
   AC_CACHE_CHECK([whether vsnprintf works],
  gmp_cv_func_vsnprintf,
   [gmp_cv_func_vsnprintf=yes
-   for i in 'return check ("hello world");' 'int n; return check ("%nhello 
world", );'; do
  AC_TRY_RUN([
 #include   /* for strcmp */
 #include/* for vsnprintf */
@@ -3713,13 +3712,12 @@ check (const char *fmt, ...)
 int
 main ()
 {
-$i
+   return check ("hello world");
 }
 ],
-  [:],
-  [gmp_cv_func_vsnprintf=no; break],
-  [gmp_cv_func_vsnprintf=probably; break])
-  done
+  [gmp_cv_func_vsnprintf=yes],
+  [gmp_cv_func_vsnprintf=no],
+  [gmp_cv_func_vsnprintf=probably])
   ])
   if test "$gmp_cv_func_vsnprintf" = probably; then
 AC_MSG_WARN([cannot check for properly working vsnprintf when cross 
compiling, will assume it's ok])

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: cc claims ISO C99 support, but %n printf format specifier calls abort()

[Theo de Raadt]

That could almost be an entry for calendars.openbsd

Dec 16 Vincent Lefevre arrives and tries to educate the OpenBSD developers
   about format string vulnerabilities, which they have been fixing
   since 1996

Vincent Lefevre  wrote:

> On 2022-12-16 09:03:39 -0700, Theo de Raadt wrote:
> > Vincent Lefevre  wrote:
> > 
> > > BTW, if developers use an untrusted format string, then sprintf()
> > > is unsafe too (possible buffer overflow), and at some point,
> > > printf() too.
> > 
> > what are you trying to say?
> 
> According to
> 
>   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2834.htm
> 
> it has been proposed to deprecate %n mainly because:
> 
>   If the format argument of a printf-style-function originates from an
>   unverified source an attacker may be able to write arbitrary values
>   to the stack.
> 
> But I'm saying that from this point of view, sprintf() is unsafe too,
> actually much more unsafe. Something like %n injection in the format
> argument can easily be detected (actually any injection of a format
> specifier). For instance, if someone writes
> 
>   printf (buf);
> 
> assuming that the string buf will be output, where buf has untrusted
> contents, then if buf contains %n, the number of arguments will be
> incorrect, so that this can be detected at run time (ditto if printf
> has arguments that correspond to a trusted part of buf). However,
> with
> 
>   sprintf (s, buf);
> 
> (rather than using snprintf), even if the number of arguments is
> checked to be correct, there could still be a buffer overflow with
> an untrusted buf.
> 
> It's a pity that N2834 doesn't give real-world examples of insecure
> use of %n, in particular ones that could not be possible to detect
> at run time.
> 
> -- 
> Vincent Lefèvre  - Web: 
> 100% accessible validated (X)HTML - Blog: 
> Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: cc claims ISO C99 support, but %n printf format specifier calls abort()

[Vincent Lefevre]

On 2022-12-16 09:03:39 -0700, Theo de Raadt wrote:
> Vincent Lefevre  wrote:
> 
> > BTW, if developers use an untrusted format string, then sprintf()
> > is unsafe too (possible buffer overflow), and at some point,
> > printf() too.
> 
> what are you trying to say?

According to

  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2834.htm

it has been proposed to deprecate %n mainly because:

  If the format argument of a printf-style-function originates from an
  unverified source an attacker may be able to write arbitrary values
  to the stack.

But I'm saying that from this point of view, sprintf() is unsafe too,
actually much more unsafe. Something like %n injection in the format
argument can easily be detected (actually any injection of a format
specifier). For instance, if someone writes

  printf (buf);

assuming that the string buf will be output, where buf has untrusted
contents, then if buf contains %n, the number of arguments will be
incorrect, so that this can be detected at run time (ditto if printf
has arguments that correspond to a trusted part of buf). However,
with

  sprintf (s, buf);

(rather than using snprintf), even if the number of arguments is
checked to be correct, there could still be a buffer overflow with
an untrusted buf.

It's a pity that N2834 doesn't give real-world examples of insecure
use of %n, in particular ones that could not be possible to detect
at run time.

-- 
Vincent Lefèvre  - Web: 
100% accessible validated (X)HTML - Blog: 
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: cc claims ISO C99 support, but %n printf format specifier calls abort()

[Theo de Raadt]

Vincent Lefevre  wrote:

> BTW, if developers use an untrusted format string, then sprintf()
> is unsafe too (possible buffer overflow), and at some point,
> printf() too.

what are you trying to say?

are you trying to say everyone including you should review and audit and
re-audit all of them?

or are you saying the opposite: that we should all _give up_ trying to fix
things, because some ISO commitees lack the balls to remove very dangerous
features which noone actually needs, and we should just accept the imperfect
world we live in?

You lost me..

Re: cc claims ISO C99 support, but %n printf format specifier calls abort()

[Theo de Raadt]

Well they need to respond, or openbsd ports needs a diff.

Vincent Lefevre  wrote:

> On 2022-12-15 18:56:15 -0700, Theo de Raadt wrote:
> > There are almost no %n left in the software ecosystem.  If we are able
> > to make this crossing, everyone else is also capable, and eventually
> > will.  Just like with gets().
> 
> FYI, this breaks GMP, whose configure script insists on %n being
> available, otherwise GMP uses its own, buggy implementation of
> vsnprintf, which triggers an assertion failure when %a/%A is used
> (and this bug affects MPFR). AFAIK, the GMP developers haven't
> reacted to the bug report sent in October.
> 
> BTW, if developers use an untrusted format string, then sprintf()
> is unsafe too (possible buffer overflow), and at some point,
> printf() too.
> 
> -- 
> Vincent Lefèvre  - Web: 
> 100% accessible validated (X)HTML - Blog: 
> Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: cc claims ISO C99 support, but %n printf format specifier calls abort()

[Stuart Henderson]

On 2022/12/16 10:50, Vincent Lefevre wrote:
> On 2022-12-15 18:56:15 -0700, Theo de Raadt wrote:
> > There are almost no %n left in the software ecosystem.  If we are able
> > to make this crossing, everyone else is also capable, and eventually
> > will.  Just like with gets().
> 
> FYI, this breaks GMP, whose configure script insists on %n being
> available, otherwise GMP uses its own, buggy implementation of
> vsnprintf, which triggers an assertion failure when %a/%A is used
> (and this bug affects MPFR). AFAIK, the GMP developers haven't
> reacted to the bug report sent in October.

btw, that doesn't appear to affect the GMP port; the values passed in from
ports infrastructure via config.cache override the autoconf check for %n
(which appears to be trying to detect a bug in Solaris 2.7 on 64-bit SPARC).

> BTW, if developers use an untrusted format string, then sprintf()
> is unsafe too (possible buffer overflow), and at some point,
> printf() too.
> 
> -- 
> Vincent Lefèvre  - Web: 
> 100% accessible validated (X)HTML - Blog: 
> Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)
> 

Re: cc claims ISO C99 support, but %n printf format specifier calls abort()

[Vincent Lefevre]

On 2022-12-15 18:56:15 -0700, Theo de Raadt wrote:
> There are almost no %n left in the software ecosystem.  If we are able
> to make this crossing, everyone else is also capable, and eventually
> will.  Just like with gets().

FYI, this breaks GMP, whose configure script insists on %n being
available, otherwise GMP uses its own, buggy implementation of
vsnprintf, which triggers an assertion failure when %a/%A is used
(and this bug affects MPFR). AFAIK, the GMP developers haven't
reacted to the bug report sent in October.

BTW, if developers use an untrusted format string, then sprintf()
is unsafe too (possible buffer overflow), and at some point,
printf() too.

-- 
Vincent Lefèvre  - Web: 
100% accessible validated (X)HTML - Blog: 
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: cc claims ISO C99 support, but %n printf format specifier calls abort()

[Theo de Raadt]

This falls into the catagory of "bummer".

We will continue to break all applications that use %n, because we
haven't found a single use of %n is that is safe. and %n uses are
completely trivial to replace.

There are almost no %n left in the software ecosystem.  If we are able
to make this crossing, everyone else is also capable, and eventually
will.  Just like with gets().

So we will not follow your proposal, because your proposal will break
thousands of other things, rather than our intented behaviour of being
safe.


The future doesn't get better if you demand backwards compatibility
forever.  The Unix landscape is getting old enough to throw bad ideas
out.


vinc...@vinc17.net wrote:

> >Synopsis:cc claims ISO C99 support, but %n printf format specifier calls 
> >abort()
> >Category:
> >Environment:
>   System  : OpenBSD 7.2
>   Details : OpenBSD 7.2 (GENERIC.MP) #2: Thu Nov 24 23:54:39 MST 2022
>
> r...@syspatch-72-amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> 
>   Architecture: OpenBSD.amd64
>   Machine : amd64
> >Description:
> 
> The compiler claims ISO C99 support (__STDC__ == 1 && __STDC_VERSION__ >= 
> 199901)
> but does not support the %n printf format specifier, which calls abort().
> 
> >How-To-Repeat:
> 
> Consider the following C program (tst.c):
> 
> #include 
> 
> int main (void)
> {
>   int n;
> 
> #if __STDC__ == 1 && __STDC_VERSION__ >= 199901
>   printf ("%nhello world", );
> #endif
>   return 0;
> }
> 
> gcc220$ cc tst.c -o tst
> tst.c:8:13: warning: '%n' format specifier support is deactivated and will 
> call abort(3)
>   printf ("%nhello world", );
>~^
> 1 warning generated.
> gcc220$ ./tst
> Abort trap (core dumped)
> 
> >Fix:
> 
> Until %n gets implemented, __STDC__ should not be set to 1.
>