security fix: arbitrary code execution with indir

[Eric Blake]

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 <address@hidden>
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 <address@hidden>
---
 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  <address@hidden>
+
+       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  <address@hidden>
 
        * 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.
+
address@hidden
+define(`%%:\
+odd', defn(`divnum'))
address@hidden
+indir(`%%:\
+odd', `extra')
address@hidden:stdin:3: Warning: %%\:\\\nodd: extra arguments ignored: 1 > 0
address@hidden
address@hidden 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