On the same day that I fixed one printf-related security hole [1, 2], I introduced another one in the very next commit [3]. Boy, I'm feeling a bit stupid right now. Fortunately, there have been no releases where indir/changeword/changesyntax can cause arbitrary code execution, only git snapshots. The patch below is for the branch; head is also affected.
[1] http://lists.gnu.org/archive/html/m4-patches/2007-11/msg00023.html [2] http://git.sv.gnu.org/gitweb/?p=m4.git;a=commitdiff;h=031a71 [3] http://git.sv.gnu.org/gitweb/?p=m4.git;a=commitdiff;h=910837#patch9 From: Eric Blake <[EMAIL PROTECTED]> Date: Wed, 6 Feb 2008 10:14:48 -0700 Subject: [PATCH] Fix security hole introduced 2007-11-22. * src/m4.h (includes): Add quotearg.h. * src/m4.c (m4_verror_at_line): Properly escape macro names. (main): Manage quoteargs defaults. * doc/m4.texinfo (Indir): Document and test this. Signed-off-by: Eric Blake <[EMAIL PROTECTED]> --- ChangeLog | 8 ++++++++ doc/m4.texinfo | 14 ++++++++++++++ src/m4.c | 30 ++++++++++++++++++++++++++---- src/m4.h | 1 + 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index eff8051..8d76e5e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2008-02-06 Eric Blake <[EMAIL PROTECTED]> + + Fix security hole introduced 2007-11-22. + * src/m4.h (includes): Add quotearg.h. + * src/m4.c (m4_verror_at_line): Properly escape macro names. + (main): Manage quoteargs defaults. + * doc/m4.texinfo (Indir): Document and test this. + 2008-02-05 Eric Blake <[EMAIL PROTECTED]> * m4/gnulib-cache.m4: Import the strtod module. diff --git a/doc/m4.texinfo b/doc/m4.texinfo index c5c7c54..dc33620 100644 --- a/doc/m4.texinfo +++ b/doc/m4.texinfo @@ -2411,6 +2411,20 @@ indir(`divert', defn(`foo')) @result{} @end example +Warning messages issued on behalf of an indirect macro use an +unambiguous representation of the macro name, using escape sequences +similar to C strings, and with colons also quoted. + [EMAIL PROTECTED] +define(`%%:\ +odd', defn(`divnum')) [EMAIL PROTECTED] +indir(`%%:\ +odd', `extra') [EMAIL PROTECTED]:stdin:3: Warning: %%\:\\\nodd: extra arguments ignored: 1 > 0 [EMAIL PROTECTED] [EMAIL PROTECTED] example + @node Builtin @section Indirect call of builtins diff --git a/src/m4.c b/src/m4.c index 2cfed19..a6bc92a 100644 --- a/src/m4.c +++ b/src/m4.c @@ -1,7 +1,7 @@ /* GNU m4 -- A simple macro processor - Copyright (C) 1989, 1990, 1991, 1992, 1993, 1994, 2004, 2005, 2006, 2007 - Free Software Foundation, Inc. + Copyright (C) 1989, 1990, 1991, 1992, 1993, 1994, 2004, 2005, 2006, + 2007, 2008 Free Software Foundation, Inc. This file is part of GNU M4. @@ -98,18 +98,37 @@ m4_verror_at_line (bool warn, int status, int errnum, const char *file, va_list args) { char *full = NULL; + char *safe_macro = NULL; + + /* Sanitize MACRO, since we are turning around and using it in a + format string. The allocation is overly conservative, but + problematic macro names only occur via indir or changeword. */ + if (macro && strchr (macro, '%')) + { + char *p = safe_macro = xcharalloc (2 * strlen (macro) + 1); + do + { + if (*macro == '%') + *p++ = '%'; + *p++ = *macro++; + } + while (*macro); + } /* Prepend warning and the macro name, as needed. But if that fails for non-memory reasons (unlikely), then still use the original format. */ if (warn && macro) - full = xasprintf (_("Warning: %s: %s"), macro, format); + full = xasprintf (_("Warning: %s: %s"), + quotearg (safe_macro ? safe_macro : macro), format); else if (warn) full = xasprintf (_("Warning: %s"), format); else if (macro) - full = xasprintf (_("%s: %s"), macro, format); + full = xasprintf (_("%s: %s"), + quotearg (safe_macro ? safe_macro : macro), format); verror_at_line (status, errnum, line ? file : NULL, line, full ? full : format, args); free (full); + free (safe_macro); if ((!warn || fatal_warnings) && !retcode) retcode = EXIT_FAILURE; } @@ -435,6 +454,8 @@ main (int argc, char *const *argv, char *const *envp) include_init (); debug_init (); + set_quoting_style (NULL, escape_quoting_style); + set_char_quoting (NULL, ':', 1); #ifdef USE_STACKOVF setup_stackovf_trap (argv, envp, stackovf_handler); #endif @@ -687,6 +708,7 @@ main (int argc, char *const *argv, char *const *envp) } output_exit (); free_regex (); + quotearg_free (); #ifdef DEBUG_REGEX if (trace_file) fclose (trace_file); diff --git a/src/m4.h b/src/m4.h index b5430d2..0f11366 100644 --- a/src/m4.h +++ b/src/m4.h @@ -43,6 +43,7 @@ #include "exitfail.h" #include "intprops.h" #include "obstack.h" +#include "quotearg.h" #include "stdio--.h" #include "stdlib--.h" #include "unistd--.h" -- 1.5.4 _______________________________________________ M4-patches mailing list [email protected] http://lists.gnu.org/mailman/listinfo/m4-patches
