Re: Format poissoning in argp-parse.c.

[Bruno Haible]

Hi,

Mats Erik Andersson wrote:
> there is a potential attack vector, and easily avoidable,
> in "lib/argp-parser.c" of GNUlib. This came to my attention
> by the modifications Guillem Jover [1] does to GNU Inetutils'
> source archives. The relevant change is reproduced below.
> (Yes, I will act myself on all those changes that only
> concern our own source!)
> 
> The problem is that the result of dgettext() is fed directly
> as format string into __argp_error(). A benevolent translator
> would produce a constant string without format escapes, but
> I know from practice that the depreciated format "%q" will
> cause OpenSolaris to segfault, so a malevolent attacker
> could in fact cause some damage.

Indeed. And the translation template of the message
"(PROGRAM ERROR) No version known!?" probably looks like this one
from libc.pot (in glibc):

#: argp/argp-parse.c:183
msgid "(PROGRAM ERROR) No version known!?"
msgstr ""

Note that it does not have a '#, c-format' annotation, therefore the
translator would not even know that she's doing something wrong if
she uses a % character in the translation. The tools would not tell her.

I'm applying this:


2012-01-06  Guillem Jover  <address@hidden>  (tiny change)

        argp: Avoid crash if translator uses % characters in a translation.
        * lib/argp-parse.c (argp_version_parser): Use a "%s" format string.
        Reported by Mats Erik Andersson <address@hidden>.

--- lib/argp-parse.c.orig       Sat Jan  7 01:37:46 2012
+++ lib/argp-parse.c    Sat Jan  7 01:34:06 2012
@@ -154,8 +154,9 @@
       else if (argp_program_version)
         fprintf (state->out_stream, "%s\n", argp_program_version);
       else
-        __argp_error (state, dgettext (state->root_argp->argp_domain,
-                                       "(PROGRAM ERROR) No version known!?"));
+        __argp_error (state, "%s",
+                      dgettext (state->root_argp->argp_domain,
+                                "(PROGRAM ERROR) No version known!?"));
       if (! (state->flags & ARGP_NO_EXIT))
         exit (0);
       break;