From owner-freebsd-current@FreeBSD.ORG Sun Apr 18 02:01:32 2010 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BB1A9106566B for ; Sun, 18 Apr 2010 02:01:32 +0000 (UTC) (envelope-from dan@dan.emsphone.com) Received: from email1.allantgroup.com (email1.emsphone.com [199.67.51.115]) by mx1.freebsd.org (Postfix) with ESMTP id 7F2418FC19 for ; Sun, 18 Apr 2010 02:01:32 +0000 (UTC) Received: from dan.emsphone.com (dan.emsphone.com [199.67.51.101]) by email1.allantgroup.com (8.14.0/8.14.0) with ESMTP id o3I21V5r054266 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Sat, 17 Apr 2010 21:01:31 -0500 (CDT) (envelope-from dan@dan.emsphone.com) Received: from dan.emsphone.com (smmsp@localhost [127.0.0.1]) by dan.emsphone.com (8.14.4/8.14.3) with ESMTP id o3I21Vjr012562 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Sat, 17 Apr 2010 21:01:31 -0500 (CDT) (envelope-from dan@dan.emsphone.com) Received: (from dan@localhost) by dan.emsphone.com (8.14.4/8.14.3/Submit) id o3I21Ur1012561; Sat, 17 Apr 2010 21:01:30 -0500 (CDT) (envelope-from dan) Date: Sat, 17 Apr 2010 21:01:30 -0500 From: Dan Nelson To: Rui Paulo Message-ID: <20100418020130.GB5413@dan.emsphone.com> References: <20100416160818.GA69460@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-OS: FreeBSD 8.0-STABLE User-Agent: Mutt/1.5.20 (2009-06-14) X-Virus-Scanned: clamav-milter 0.96 at email1.allantgroup.com X-Virus-Status: Clean X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-2.0.2 (email1.allantgroup.com [199.67.51.78]); Sat, 17 Apr 2010 21:01:31 -0500 (CDT) X-Scanned-By: MIMEDefang 2.45 Cc: freebsd-current@freebsd.org, Ivan Voras Subject: Re: [CFT]: ClangBSD is selfhosting, we need testers now X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 18 Apr 2010 02:01:32 -0000 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@allantgroup.com