Re: Build failures on Ubuntu due to gettext

[Colin Watson]

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                                       address@hidden