Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 7 Apr 2017 10:26:29 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: Undefined behavior in sprintf(dest, "%lld", -1LL)

On Fri, Apr 07, 2017 at 02:22:46PM +0000, Pascal Cuoq wrote:
> I am running musl's implementation inside tis-interpreter, a C interpreter that detects a very wide palette of undefined behaviors.
> 
> When musl's printf implementation is invoked through the following test driver:
> 
>   char dest[100];
>   int x = sprintf(dest, "%lld\n", -1LL);
> 
> tis-interpreter emits this warning:
> 
> src/stdio/vfprintf.c:141: warning: although the type provided to the va_arg macro (unsigned long long) and the actual type of the next variadic argument (long long) are corresponding unsigned - signed variants of the same integer type, the actual value of the argument (signed) is negative thus it cannot be represented in the provided (unsigned) type
> 
> The warning of course refers to C11 7.16.1.1:2 http://port70.net/~nsz/c/c11/n1570.html#7.16.1.1p2
> 
> To get a clearer view, I instrumented the code thus using the debug function tis_printf (we are inside the implementation of printf, it is a bad idea to call printf here):
> 
> --- ../musl-1.1.16/src/stdio/vfprintf.c 2017-01-01 04:27:17.000000000 +0100
> +++ src/stdio/vfprintf.c 2017-04-07 16:13:19.291592002 +0200
> @@ -547,7 +547,7 @@
>   if (argpos>=0) goto inval;
>   } else {
>   if (argpos>=0) nl_type[argpos]=st, arg=nl_arg[argpos];
> - else if (f) pop_arg(&arg, st, ap);
> + else if (f) { tis_printf("st:%u LLONG:%u ULLONG:%u\n", st, LLONG, ULLONG); pop_arg(&arg, st, ap); }
>   else return 0;
>   }
> 
> This shows the debug message below:
> 
> st:12 LLONG:12 ULLONG:12
> 
> On the basis of this result, I would tentatively offer that the undefined behavior that tis-interpreter warns about is real and is caused by line 55 in vfprintf.c:
> 
> #define LLONG ULLONG
> 
> Because of this approach, when LLONG is passed to the function pop_arg(), the switch case at line 141 is taken:
> 
> break; case ULLONG: arg->i = va_arg(*ap, unsigned long long);
> 
> This causes UB by consuming a (negative) long long argument from a va_list with va_arg(..., unsigned long long).
> 
> Pascal
> 

Does defining the ODD_TYPES macro fix the problem? My leaning is to
just remove that #ifdef logic and always use the correct type with
va_arg. All that was doing was saving a few bytes of code; the change
should not affect performance.

Rich

Powered by blists - more mailing lists

Your e-mail address:

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.