Re: [CFT]: ClangBSD is selfhosting, we need testers now

In the last episode (Apr 17), Rui Paulo said:
> On 16 Apr 2010, at 22:41, Ivan Voras wrote:
> > I have a buildworld error here:
> > 
> > clang -isystem /usr/obj/mt/clangbsd/tmp/usr/include/clang/1.5 -isystem /usr/obj/mt/clangbsd/tmp/usr/include -B/usr/obj/mt/clangbsd/tmp/usr/lib/ -L/usr/obj/mt/clangbsd/tmp/usr/lib/ -fpic -DPIC -O2 -pipe -mtune=generic  -I/mt/clangbsd/lib/libc/include -I/mt/clangbsd/lib/libc/../../include -I/mt/clangbsd/lib/libc/amd64 -DNLS -D__DBINTERFACE_PRIVATE -I/mt/clangbsd/lib/libc/../../contrib/gdtoa -DINET6 -I/usr/obj/mt/clangbsd/lib/libc -I/mt/clangbsd/lib/libc/resolv -D_ACL_PRIVATE -DPOSIX_MISTAKE -I/mt/clangbsd/lib/libc/../../contrib/tzcode/stdtime -I/mt/clangbsd/lib/libc/stdtime -I/mt/clangbsd/lib/libc/locale -DBROKEN_DES -DPORTMAP -DDES_BUILTIN -I/mt/clangbsd/lib/libc/rpc -DYP -DNS_CACHING -DSYMBOL_VERSIONING -std=gnu99 -fstack-protector -Wsystem-headers -Werror -Wall -Wno-format-y2k -Wno-uninitialized -Wno-pointer-sign -c /mt/clangbsd/lib/libc/sys/__error.c -o __error.So
> > /mt/clangbsd/lib/libc/sys/stack_protector.c:88:19: error: format string is not a string literal (potentially insecure) [-Wformat-security]
> >        syslog(LOG_CRIT, msg);
> >                         ^~~
> > 1 diagnostic generated.
> > *** Error code 1
> > 2 errors
> > *** Error code 2
> > 1 error
> > 
> > The context is... I think a bit overprotective here :) At least this
> > particular warning knob should probably be turned off.
> 
> Actually, I would rather fix the code that does this than disabling the
> warning.  Even if this particular code is not vulnerable to format string
> problems, it's 2010 now and it doesn't hurt to add a "%s" there.

Agree.  Calling sprintf-like functions with unknown format strings is
dangerous.  Technically, though, the code as is stands is safe, since __fail
is static and the only two callers do pass safe string literals.

It looks like we found three buglets here :)

1) __fail should use syslog(LOG_CRIT, "%s", msg).

2) We should add -Wformat-nonliteral to the gcc CFLAGS list so it can make
   the same check across the entire build (though it's no smarter than clang
   and emits one for this file).

3) Both clang and gcc (when asked to) emit a warning when they have enough
   information to know it's safe.  It's better to err on the side of
   caution, though, and the compiler would have to analyze the whole source
   file to know that all the callers use the function safely.

-- 
	Dan Nelson
	dnelson_at_allantgroup.com