Re: debian bug 5898 - security option

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Eric Blake on 7/31/2006 9:04 PM:
> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=5898
> 
> Yes, you read that right - an open bug with only a 4-digit ID.  9 years
> and 236 days old.
> 
> The idea might be nice for m4 2.0, but is not worth adding to the 1.4.x
> branch.  My take on what a --secure option would disable:
> 
> debugfile (it can overwrite arbitrary existing files)
> syscmd (it invokes arbitrary shell commands)
> esyscmd (likewise)
> maketemp (invoked enough times, it can form a denial-of-service by
> creating lots of files)
> builtin (at least, builtin on any of the restricted commands)

And here is my first cut at this patch, which will be in the eventual m4
2.0.  The door is open for using -S with no arguments as a short option
for --safer, but I did not make that change now, since it is incompatible
with the existing (but intentionally undocumented) -S<int> option for
compatibility with Solaris m4.

2006-09-14  Eric Blake  <address@hidden>

        Add --safer option, per debian bug 5898.
        * src/main.c (usage): Document new option.
        (SAFER_OPTION): New enumerator.
        (main): Set the option bit.
        * m4/m4module.h (m4_context_opt_bit_table): Declare new bit
        accessors.
        * m4/m4private.h (M4_OPT_SAFER_BIT): New macro.
        (m4_get_safer_opt): New accessor.
        * modules/gnu.c (esyscmd, debugfile): Disable when --safer.
        * modules/m4.c (syscmd, maketemp): Likewise.
        * doc/m4.texinfo (Invoking m4, Debug Output, Syscmd, Esyscmd)
        (Sysval, Maketemp): Add tests of this.
        * tests/options.at (--safer): Likewise.

- --
Life is short - so eat dessert first!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFFCiAf84KuGfSFAYARAgIlAKCF1+5QwGg+yrmLgzLV0sI4yLvuGACfYoCX
m/RL8MWDYLfsKVv5qmhWO48=
=7x/0
-----END PGP SIGNATURE-----

Index: doc/m4.texinfo
===================================================================
RCS file: /sources/m4/m4/doc/m4.texinfo,v
retrieving revision 1.43
diff -u -p -r1.43 m4.texinfo
--- doc/m4.texinfo      7 Sep 2006 23:53:04 -0000       1.43
+++ doc/m4.texinfo      15 Sep 2006 03:30:21 -0000
@@ -463,6 +463,14 @@ Set the regular expression syntax accord
 When this option is not given, @acronym{GNU} M4 uses emacs compatible
 regular expressions.  @xref{Changeresyntax}, for more details on the
 format and meaning of @var{RESYNTAX-SPEC}.
+
address@hidden --safer
+Cripple the builtins @code{maketemp} (@pxref{Maketemp}),
address@hidden (@pxref{Debug Output}), @code{syscmd} (@pxref{Syscmd}),
+and @code{esyscmd} (@pxref{Esyscmd}), since they can perform potentially
+unsafe actions.  An attempt to use these macros will result in an error.
+This option is intended to make it safer to preprocess an input file of
+unknown origin.
 @end table
 
 On platforms that support dynamic libraries, there are some options
@@ -2699,13 +2707,30 @@ Debug and tracing output can be redirect
 @samp{-o} option to @code{m4}, or with the builtin macro @code{debugfile}:
 
 @deffn {Builtin (gnu)} debugfile
address@hidden {Builtin (gnu)} debugfile @w{(opt @var{filename})}
-Send all further debug and trace output to @var{filename}.  If
address@hidden is empty, debug and trace output are discarded and if
address@hidden is called without any arguments, debug and trace output
-are sent to the standard error output.
address@hidden {Builtin (gnu)} debugfile @w{(opt @var{file})}
+Send all further debug and trace output to @var{file}, opened in append
+mode.  If @var{file} is the empty string, debug and trace output are
+discarded and if @code{debugfile} is called without any arguments, debug
+and trace output are sent to the standard error output.
+
+When the @option{--safer} option (@pxref{Invoking m4}) is in effect,
address@hidden must be empty or omitted, since otherwise an input file
+could cause the modification of arbitrary files.
 @end deffn
 
address@hidden options: --safer
address@hidden status: 1
address@hidden
+$ @kbd{m4 --safer}
+debugfile(`foo')
address@hidden:stdin:1: debugfile: disabled by --safer
address@hidden
+debugfile()
address@hidden
+debugfile
address@hidden
address@hidden example
+
 @node Input Control
 @chapter Input control
 
@@ -4435,6 +4460,10 @@ Prior to executing the command, @code{m4
 The default standard input, output and error of @var{shell-command} are
 the same as those of @code{m4}.
 
+When the @option{--safer} option (@pxref{Invoking m4}) is in effect,
address@hidden results in an error, since otherwise an input file could
+execute arbitrary code.
+
 The builtin macro @code{syscmd} is recognized only when given arguments.
 @end deffn
 
@@ -4449,7 +4478,14 @@ syscmd(`echo foo')
 Note how the expansion of @code{syscmd} keeps the trailing newline of
 the command, as well as using the newline that appeared after the macro.
 
-The builtin macro @code{syscmd} is recognized only when given arguments.
address@hidden options: --safer
address@hidden status: 1
address@hidden
+$ @kbd{m4 --safer}
+syscmd(`echo hi')
address@hidden:stdin:1: syscmd: disabled by --safer
address@hidden
address@hidden example
 
 @node Esyscmd
 @section Reading the output of commands
@@ -4478,10 +4514,23 @@ esyscmd(`echo foo')
 Note how the expansion of @code{esyscmd} keeps the trailing newline of
 the command, as well as using the newline that appeared after the macro.
 
+When the @option{--safer} option (@pxref{Invoking m4}) is in effect,
address@hidden results in an error, since otherwise an input file could
+execute arbitrary code.
+
 The builtin macro @code{esyscmd} is recognized only when given
 arguments.
 @end deffn
 
address@hidden options: --safer
address@hidden status: 1
address@hidden
+$ @kbd{m4 --safer}
+esyscmd(`echo hi')
address@hidden:stdin:1: esyscmd: disabled by --safer
address@hidden
address@hidden example
+
 @node Sysval
 @section Exit status
 
@@ -4506,6 +4555,25 @@ sysval
 @result{}0
 @end example
 
+
+When the @option{--safer} option (@pxref{Invoking m4}) is in effect,
address@hidden will always remain at its default value of zero.
+
address@hidden options: --safer
address@hidden status: 1
address@hidden
+$ @kbd{m4 --safer}
+sysval
address@hidden
+syscmd(`false')
address@hidden:stdin:2: syscmd: disabled by --safer
address@hidden
+sysval
address@hidden
address@hidden example
+
+
+
 @node Maketemp
 @section Making temporary files
 
@@ -4528,10 +4596,24 @@ maketemp(`/tmp/fooXXXXXX')
 @result{}/tmp/fooa07346
 @end example
 
+When the @option{--safer} option (@pxref{Invoking m4}) is in effect,
address@hidden results in an error, since otherwise an input file could
+perform a mild denial-of-service attack by filling up a disk with
+multiple empty files.
+
 The builtin macro @code{maketemp} is recognized only when given
 arguments.
 @end deffn
 
address@hidden options: --safer
address@hidden status: 1
address@hidden
+$ @kbd{m4 --safer}
+maketemp(`/tmp/fooXXXXXX')
address@hidden:stdin:1: maketemp: disabled by --safer
address@hidden
address@hidden example
+
 @node Miscellaneous
 @chapter Miscellaneous builtin macros
 
Index: m4/m4module.h
===================================================================
RCS file: /sources/m4/m4/m4/m4module.h,v
retrieving revision 1.84
diff -u -p -r1.84 m4module.h
--- m4/m4module.h       7 Sep 2006 23:53:04 -0000       1.84
+++ m4/m4module.h       15 Sep 2006 03:30:21 -0000
@@ -151,6 +151,7 @@ extern void         m4_delete       (m4 *);
        M4OPT_BIT(M4_OPT_SYNC_OUTPUT_BIT,       sync_output_opt)        \
        M4OPT_BIT(M4_OPT_POSIXLY_CORRECT_BIT,   posixly_correct_opt)    \
        M4OPT_BIT(M4_OPT_FATAL_WARN_BIT,        fatal_warnings_opt)     \
+       M4OPT_BIT(M4_OPT_SAFER_BIT,             safer_opt)              \
 
 
 #define M4FIELD(type, base, field)                                     \
Index: m4/m4private.h
===================================================================
RCS file: /sources/m4/m4/m4/m4private.h,v
retrieving revision 1.60
diff -u -p -r1.60 m4private.h
--- m4/m4private.h      7 Sep 2006 23:53:04 -0000       1.60
+++ m4/m4private.h      15 Sep 2006 03:30:21 -0000
@@ -82,6 +82,7 @@ struct m4 {
 #define M4_OPT_SYNC_OUTPUT_BIT         (1 << 4) /* -s */
 #define M4_OPT_POSIXLY_CORRECT_BIT     (1 << 5) /* POSIXLY_CORRECT */
 #define M4_OPT_FATAL_WARN_BIT          (1 << 6) /* -E */
+#define M4_OPT_SAFER_BIT               (1 << 7) /* --safer */
 
 /* Fast macro versions of accessor functions for public fields of m4,
    that also have an identically named function exported in m4module.h.  */
@@ -125,6 +126,8 @@ struct m4 {
                (BIT_TEST((C)->opt_flags, M4_OPT_POSIXLY_CORRECT_BIT))
 #  define m4_get_fatal_warnings_opt(C)                                 \
                (BIT_TEST((C)->opt_flags, M4_OPT_FATAL_WARN_BIT))
+#  define m4_get_safer_opt(C)                                          \
+               (BIT_TEST((C)->opt_flags, M4_OPT_SAFER_BIT))
 
 /* No fast opt bit set macros, as they would need to evaluate their
    arguments more than once, which would subtly change their semantics.  */
Index: modules/gnu.c
===================================================================
RCS file: /sources/m4/m4/modules/gnu.c,v
retrieving revision 1.54
diff -u -p -r1.54 gnu.c
--- modules/gnu.c       5 Sep 2006 13:25:24 -0000       1.54
+++ modules/gnu.c       15 Sep 2006 03:30:21 -0000
@@ -400,6 +400,8 @@ M4BUILTIN_HANDLER (debugfile)
 {
   if (argc == 1)
     m4_debug_set_output (context, NULL);
+  else if (m4_get_safer_opt (context) && *M4ARG (1))
+    m4_error (context, 0, 0, _("%s: disabled by --safer"), M4ARG (0));
   else if (!m4_debug_set_output (context, M4ARG (1)))
     m4_error (context, 0, errno, _("%s: cannot set debug file: %s"),
              M4ARG (0), M4ARG (1));
@@ -449,12 +451,18 @@ M4BUILTIN_HANDLER (esyscmd)
       FILE *pin;
       int ch;
 
+      if (m4_get_safer_opt (context))
+       {
+         m4_error (context, 0, 0, _("%s: disabled by --safer"), M4ARG (0));
+         return;
+       }
+
       /* Optimize the empty command.  */
       if (*M4ARG (1) == '\0')
-        {
-          m4_set_sysval (0);
-          return;
-        }
+       {
+         m4_set_sysval (0);
+         return;
+       }
 
       m4_sysval_flush (context);
       errno = 0;
Index: modules/m4.c
===================================================================
RCS file: /sources/m4/m4/modules/m4.c,v
retrieving revision 1.69
diff -u -p -r1.69 m4.c
--- modules/m4.c        7 Sep 2006 23:53:04 -0000       1.69
+++ modules/m4.c        15 Sep 2006 03:30:21 -0000
@@ -495,7 +495,13 @@ m4_sysval_flush (m4 *context)
 
 M4BUILTIN_HANDLER (syscmd)
 {
-  /* Optimize the empty command.  */
+   if (m4_get_safer_opt (context))
+   {
+     m4_error (context, 0, 0, _("%s: disabled by --safer"), M4ARG (0));
+     return;
+   }
+
+   /* Optimize the empty command.  */
   if (*M4ARG (1) == '\0')
     {
       m4_set_sysval (0);
@@ -512,8 +518,8 @@ M4BUILTIN_HANDLER (syscmd)
 M4BUILTIN_HANDLER (sysval)
 {
   m4_shipout_int (obs, (m4_sysval == -1 ? 127
-                        : (M4_SYSVAL_EXITBITS (m4_sysval)
-                           | M4_SYSVAL_TERMSIGBITS (m4_sysval))));
+                       : (M4_SYSVAL_EXITBITS (m4_sysval)
+                          | M4_SYSVAL_TERMSIGBITS (m4_sysval))));
 }
 
 
@@ -674,6 +680,12 @@ M4BUILTIN_HANDLER (maketemp)
 {
   int fd;
 
+  if (m4_get_safer_opt (context))
+    {
+      m4_error (context, 0, 0, _("%s: disabled by --safer"), M4ARG (0));
+      return;
+    }
+
   errno = 0;
   if ((fd = mkstemp (M4ARG(1))) < 0)
     {
Index: src/main.c
===================================================================
RCS file: /sources/m4/m4/src/main.c,v
retrieving revision 1.81
diff -u -p -r1.81 main.c
--- src/main.c  14 Sep 2006 00:37:26 -0000      1.81
+++ src/main.c  15 Sep 2006 03:30:21 -0000
@@ -79,6 +79,8 @@ for short options too.\n\
 Operation modes:\n\
       --help                   display this help and exit\n\
       --version                output version information and exit\n\
+"), stdout);
+      fputs (_("\
   -b, --batch                  buffer output, process interrupts\n\
   -c, --discard-comments       do not copy comments to the output\n\
   -E, --fatal-warnings         stop execution after first warning\n\
@@ -86,6 +88,7 @@ Operation modes:\n\
   -P, --prefix-builtins        force a `m4_' prefix to all builtins\n\
   -Q, --quiet, --silent        suppress some warnings for builtins\n\
   -r, --regexp-syntax=SPEC     change the default regexp syntax\n\
+  -s, --safer                  disable potentially unsafe builtins\n\
 "), stdout);
       fputs (_("\
 \n\
@@ -172,6 +175,7 @@ enum
   DIVERSIONS_OPTION = CHAR_MAX + 1,    /* not quite -N, because of message */
   IMPORT_ENVIRONMENT_OPTION,           /* no short opt */
   PREPEND_INCLUDE_OPTION,              /* not quite -B, because of message */
+  SAFER_OPTION,                                /* -S still has old no-op 
semantics */
 
   HELP_OPTION,                         /* no short opt */
   VERSION_OPTION                       /* no short opt */
@@ -208,6 +212,7 @@ static const struct option long_options[
   {"diversions", required_argument, NULL, DIVERSIONS_OPTION},
   {"import-environment", no_argument, NULL, IMPORT_ENVIRONMENT_OPTION},
   {"prepend-include", required_argument, NULL, PREPEND_INCLUDE_OPTION},
+  {"safer", no_argument, NULL, SAFER_OPTION},
 
   {"help", no_argument, NULL, HELP_OPTION},
   {"version", no_argument, NULL, VERSION_OPTION},
@@ -424,6 +429,10 @@ main (int argc, char *const *argv, char 
        import_environment = true;
        break;
 
+      case SAFER_OPTION:
+       m4_set_safer_opt (context, true);
+        break;
+
       case VERSION_OPTION:
        version_etc (stdout, PACKAGE, PACKAGE_NAME TIMESTAMP,
                     VERSION, AUTHORS, NULL);
Index: tests/options.at
===================================================================
RCS file: /sources/m4/m4/tests/options.at,v
retrieving revision 1.7
diff -u -p -r1.7 options.at
--- tests/options.at    5 Sep 2006 23:16:40 -0000       1.7
+++ tests/options.at    15 Sep 2006 03:30:21 -0000
@@ -230,3 +230,47 @@ AT_CHECK([[sed -n -e 's|There is NO WARR
 ])
 
 AT_CLEANUP
+
+
+## ----- ##
+## safer ##
+## ----- ##
+
+AT_SETUP([--safer])
+
+dnl with --safer, debugfile is crippled, but -o is not
+AT_DATA([[in]],
+[[define(`foo', `1')foo
+debugfile(`trace2')
+define(`foo', `2')foo
+]])
+
+AT_CHECK_M4([--safer -o trace1 -t foo in], [1],
+[[1
+
+2
+]], [[m4:in:2: debugfile: disabled by --safer
+]])
+
+AT_CHECK([test -f trace2], [1])
+AT_CHECK([cat trace1], [0],
+[[m4trace: -1- foo -> `1'
+m4trace: -1- foo -> `2'
+]])
+
+dnl make sure builtin cannot bypass --safer, and that maketemp does not
+dnl create file
+AT_DATA([[in]], [[builtin(`maketemp', `./fooXXXXXX')
+]])
+
+AT_CHECK([echo foo*], [0], [foo*
+])
+
+AT_CHECK_M4([--safer in], [1], [[
+]], [[m4:in:1: maketemp: disabled by --safer
+]])
+
+AT_CHECK([echo foo*], [0], [foo*
+])
+
+AT_CLEANUP