From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1NHkbG-0001TY-3x for mharc-grub-devel@gnu.org; Mon, 07 Dec 2009 15:54:30 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NHkbE-0001S1-7N for grub-devel@gnu.org; Mon, 07 Dec 2009 15:54:28 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NHkb9-0001NL-1Y for grub-devel@gnu.org; Mon, 07 Dec 2009 15:54:27 -0500 Received: from [199.232.76.173] (port=57866 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NHkb8-0001N9-N0 for grub-devel@gnu.org; Mon, 07 Dec 2009 15:54:22 -0500 Received: from smarthost03.mail.zen.net.uk ([212.23.3.142]:36509) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NHkb8-0003o2-4C for grub-devel@gnu.org; Mon, 07 Dec 2009 15:54:22 -0500 Received: from [82.69.40.219] (helo=riva.pelham.vpn.ucam.org) by smarthost03.mail.zen.net.uk with esmtp (Exim 4.63) (envelope-from ) id 1NHkb7-00014c-5H for grub-devel@gnu.org; Mon, 07 Dec 2009 20:54:21 +0000 Received: from cjwatson by riva.pelham.vpn.ucam.org with local (Exim 3.36 #1 (Debian)) for grub-devel@gnu.org id 1NHkb6-0001dc-00; Mon, 07 Dec 2009 20:54:20 +0000 Date: Mon, 7 Dec 2009 20:54:20 +0000 From: Colin Watson To: The development of GNU GRUB Message-ID: <20091207205420.GI6439@riva.ucam.org> References: <20091207140923.GA6439@riva.ucam.org> <20091207201408.GA27276@pina.cat> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20091207201408.GA27276@pina.cat> User-Agent: Mutt/1.5.18 (2008-05-17) X-Originating-Smarthost03-IP: [82.69.40.219] X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6 (newer, 3) Subject: Re: Build failures on Ubuntu due to gettext X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Dec 2009 20:54:28 -0000 On Mon, Dec 07, 2009 at 08:14:08PM +0000, Carles Pina i Estany wrote: > On Dec/07/2009, Colin Watson wrote: > > Ubuntu's GCC enables -Wformat-security by default. This causes GCC to > > (IMO rightly!) complain about constructs such as this: > > > > grub_printf (_("foo")); > > I see... > (actually some weeks ago I thought about gettext security implications, > and I thought that if someone can change the .mo files there is some > bigger problem for the user... but I understand the point) > > > ... because it's all too easy for a translator to (usually > > accidentally) insert % sequences which would cause printf to behave > > (side and non-relevant note: if the translator wants to do some damage on > purpose he doesn't need to play with %s and %d, it's as easy as changing > strings from "Press C to Cancel and D to delete" to something like (in another > language) "Press D to Cancel and C to delete") > > > incorrectly. This should instead be: > > > > grub_printf ("%s", _("foo")); > > I like the general idea but I'm not 100% convinced of the > implementation: > > a) It's a bit of false security because it's not fixing the case of file > normal/menu_text.c, line 191 (search the string "Use the %C and %C keys > to") or menu_text.c line 374 (search for "The highlighted entry") > (I agree that it's better than before... but not very solid) Forget security, in this case; let's just think of it as a way to avoid translators accidentally shooting themselves (and their users) in the foot. I have seen this happen and it's best to prevent it by static checking. > b) How would you translate and handle: > grub_printf (_("Hello %s"), name); -Wformat-security doesn't interfere with that. As its documentation says: At present, this warns about calls to `printf' and `scanf' functions where the format string is not a string literal and there are no format arguments, as in `printf (foo);'. > c) I'm thinking to implement grub_printf_ (str). I will send a patch > later to see if everybody likes. Then the call for simple strings would > be: > grub_printf_ ("%s", N_("Hello")); > > Not much different from: > grub_printf_ (N_("Hello")); > > But it's getting hard to print a string :-) grub_puts would be nice and would match the POSIX API (or maybe grub_fputs or grub_putstr or something, since POSIX puts is kind of odd in writing a trailing newline). I don't really like significant trailing underscores in function names myself, although I see why you chose it here. > d) (important, even if it's the last one): > How it prevents mistakes from the current msgfmt checks? > For example, and using the option -c in msgfmt: > #: normal/misc.c:67 > #, c-format > msgid "test %s t" > msgstr "test %d test2" > > /usr/bin/msgfmt -c --statistics -o po/ca.mo po/ca.po > po/ca.po:1183: format specifications in 'msgid' and 'msgstr' for argument 1 are > not the same > /usr/bin/msgfmt: found 1 fatal error > 26 translated messages, 213 untranslated messages. > > Using any different number of %X from msgid and msgstr ishalting msgfmt (so, if > msgid contains 1 %s and 2 %d, msgstr has to contain the same) That only works for strings with the "c-format" attribute, which xgettext only applies (and should only apply) to msgids that contain "%". It helps for the string you mentioned earlier with %C in the msgid, but it's no use for _("literal string with no percent characters"). > We are talking from _(" "), I see that -Wformat-security is for "string came > from untrusted input and contains" when .mo are trusted. Are they? I would recommend against considering .mo as entirely trusted, unless you really think that developers will read every submission line-by-line. I realise that as you say there are plenty of more subtle avenues of attack; as I said above, I really think of this as foot-shooting prevention more than security. > How are other projectes implementing it? Specially the dynamic strings. Every competent project I've seen that uses gettext in C is in line with the patch I sent, although some use an equivalent of printf("%s", ...) while some use an equivalent of fputs(..., stdout). > I like the idea and I understand that it's easy to make mistakes > translating strings. Actually I'm surprised that msgfmt is not giving > any warning if the In general it can only do this for appropriately tagged strings. For an arbitrary literal string it often isn't actually wrong to translate it with text containing a "%" sign - it's just that it crashes the program. Thus, it's the program's responsibility to avoid crashing. -- Colin Watson [cjwatson@ubuntu.com]