This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "GNU M4 source repository".
http://git.sv.gnu.org/gitweb/?p=m4.git;a=commitdiff;h=bd9900d65eb9cd5add0f107e94b513fa267495ba The branch, branch-1_4 has been updated via bd9900d65eb9cd5add0f107e94b513fa267495ba (commit) from 83e9a157eb78afe47c7955a6fa99e0de79a8b40a (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- commit bd9900d65eb9cd5add0f107e94b513fa267495ba Author: Eric Blake <[EMAIL PROTECTED]> Date: Fri Dec 7 11:55:18 2007 -0700 Minor security fix: Quote output of mkstemp. * src/builtin.c (mkstemp_helper): Produce quoted output. * doc/m4.texinfo (Mkstemp): Update the documentation and tests. * NEWS: Document this change. Signed-off-by: Eric Blake <[EMAIL PROTECTED]> ----------------------------------------------------------------------- Summary of changes: ChangeLog | 5 ++++ NEWS | 5 ++++ doc/m4.texinfo | 57 +++++++++++++++++++++++++++++++++++++++++++------------ src/builtin.c | 32 ++++++++++++++++-------------- 4 files changed, 71 insertions(+), 28 deletions(-) diff --git a/ChangeLog b/ChangeLog index 79a133a..8838ac5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ 2007-12-07 Eric Blake <[EMAIL PROTECTED]> + Minor security fix: Quote output of mkstemp. + * src/builtin.c (mkstemp_helper): Produce quoted output. + * doc/m4.texinfo (Mkstemp): Update the documentation and tests. + * NEWS: Document this change. + Stage 5: add notion of quote age. * src/input.c: Comment cleanups. (current_quote_age): New global variable. diff --git a/NEWS b/NEWS index 1762571..051a16a 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,11 @@ Version 1.4.11 - ?? ??? 2007, by ???? (git version 1.4.10a-*) * Fix regression introduced in 1.4.9b in the `divert' builtin when more than 512 kibibytes are saved in diversions on platforms like NetBSD where fopen(name,"a+") seeks to the end of the file. +* The output of the `maketemp' and `mkstemp' builtins is now quoted if a + file was created. This is a minor security fix, because it was possible + (although rather unlikely) that an unquoted string could match an + existing macro name, such that use of the `mkstemp' output would trigger + inadvertent macro expansion and operate on the wrong file name. * Enhance the `defn' builtin to support concatenation of multiple text arguments, as required by POSIX. However, at this time, it is not possible to concatenate a builtin macro with anything else; a warning is diff --git a/doc/m4.texinfo b/doc/m4.texinfo index 803dbf0..93b7696 100644 --- a/doc/m4.texinfo +++ b/doc/m4.texinfo @@ -5876,7 +5876,7 @@ builtin macro, @code{mkstemp}, for making a temporary file: @deffn Builtin mkstemp (@var{template}) @deffnx Builtin maketemp (@var{template}) -Expands to a name of a new, empty file, made from the string +Expands to the quoted name of a new, empty file, made from the string @var{template}, which should end with the string @samp{XXXXXX}. The six @samp{X} characters are then replaced with random characters matching the regular expression @samp{[a-zA-Z0-9._-]}, in order to make the file @@ -5888,7 +5888,8 @@ account, and at most only the current user can read and write the file. The traditional behavior, standardized by @acronym{POSIX}, is that @code{maketemp} merely replaces the trailing @samp{X} with the process -id, without creating a file, and without ensuring that the resulting +id, without creating a file or quoting the expansion, and without +ensuring that the resulting string is a unique file name. In part, this means that using the same @var{template} twice in the same input file will result in the same expansion. This behavior is a security hole, as it is very easy for @@ -5912,6 +5913,8 @@ chosen: @comment ignore @example $ @kbd{m4} +define(`tmp', `oops') [EMAIL PROTECTED] maketemp(`/tmp/fooXXXXXX') @result{}/tmp/fooa07346 ifdef(`mkstemp', `define(`maketemp', defn(`mkstemp'))', @@ -5929,26 +5932,35 @@ Unless you use the @option{--traditional} command line option (or version of @code{maketemp} is secure. This means that using the same template to multiple calls will generate multiple files. However, we recommend that you use the new @code{mkstemp} macro, introduced in [EMAIL PROTECTED] M4 1.4.8, which is secure even in traditional mode. [EMAIL PROTECTED] M4 1.4.8, which is secure even in traditional mode. Also, +as of M4 1.4.11, the secure implementation quotes the resulting file +name, so that you are guaranteed to know what file was created even if +the random file name happens to match an existing macro. Notice that +this example is careful to use @code{defn} to avoid unintended expansion +of @samp{foo}. @example $ @kbd{m4} -syscmd(`rm -f foo??????')sysval +define(`foo', `errprint(`oops')') [EMAIL PROTECTED] +syscmd(`rm -f foo-??????')sysval @result{}0 -define(`file1', maketemp(`fooXXXXXX'))dnl -ifelse(esyscmd(`echo foo??????'), `foo??????', `no file', `created') +define(`file1', maketemp(`foo-XXXXXX'))dnl +ifelse(esyscmd(`echo \` foo-?????? \''), ` foo-?????? ', + `no file', `created') @result{}created -define(`file2', maketemp(`fooXX'))dnl -define(`file3', mkstemp(`fooXXXXXX'))dnl -ifelse(len(file1), len(file2), `same length', `different') +define(`file2', maketemp(`foo-XX'))dnl +define(`file3', mkstemp(`foo-XXXXXX'))dnl +ifelse(len(defn(`file1')), len(defn(`file2')), + `same length', `different') @result{}same length -ifelse(file1, file2, `same', `different file') +ifelse(defn(`file1'), defn(`file2'), `same', `different file') @result{}different file -ifelse(file2, file3, `same', `different file') +ifelse(defn(`file2'), defn(`file3'), `same', `different file') @result{}different file -ifelse(file1, file3, `same', `different file') +ifelse(defn(`file1'), defn(`file3'), `same', `different file') @result{}different file -syscmd(`rm 'file1 file2 file3) +syscmd(`rm 'defn(`file1') defn(`file2') defn(`file3')) @result{} sysval @result{}0 @@ -5966,6 +5978,25 @@ len(mkstemp(`fooXXXXX')) syscmd(`rm foo??????')sysval @result{}0 @end example + [EMAIL PROTECTED] Likewise, and ensure that traditional mode leaves the result unquoted [EMAIL PROTECTED] without creating a file. + [EMAIL PROTECTED] options: -G [EMAIL PROTECTED] +syscmd(`rm -f foo-*')sysval [EMAIL PROTECTED] +len(maketemp(`foo-XXXXX')) [EMAIL PROTECTED]:stdin:2: Warning: maketemp: recommend using mkstemp instead [EMAIL PROTECTED] +define(`abc', `def') [EMAIL PROTECTED] +maketemp(`foo-abc') [EMAIL PROTECTED] [EMAIL PROTECTED]:stdin:4: Warning: maketemp: recommend using mkstemp instead +syscmd(`test -f foo-*')sysval [EMAIL PROTECTED] [EMAIL PROTECTED] example @end ignore @node Miscellaneous diff --git a/src/builtin.c b/src/builtin.c index b4053f1..87f8c2f 100644 --- a/src/builtin.c +++ b/src/builtin.c @@ -1433,40 +1433,42 @@ m4_sinclude (struct obstack *obs, int argc, macro_arguments *argv) | Use the first argument as a template for a temporary file name. | `-----------------------------------------------------------------*/ -/* Add trailing 'X' to NAME of length LEN as necessary, then securely - create the file, and place the new file name on OBS. Report errors - on behalf of ME. */ +/* Add trailing 'X' to PATTERN of length LEN as necessary, then + securely create the file, and place the quoted new file name on + OBS. Report errors on behalf of ME. */ static void -mkstemp_helper (struct obstack *obs, const char *me, const char *name, +mkstemp_helper (struct obstack *obs, const char *me, const char *pattern, size_t len) { int fd; int i; + char *name; /* Guarantee that there are six trailing 'X' characters, even if the - user forgot to supply them. */ - obstack_grow (obs, name, len); + user forgot to supply them. Output must be quoted if + successful. */ + obstack_grow (obs, lquote.string, lquote.length); + obstack_grow (obs, pattern, len); for (i = 0; len > 0 && i < 6; i++) - if (name[--len] != 'X') + if (pattern[len - i - 1] != 'X') break; - for (; i < 6; i++) - obstack_1grow (obs, 'X'); - obstack_1grow (obs, '\0'); + len += 6 - i; + obstack_grow0 (obs, "XXXXXX", 6 - i); + name = (char *) obstack_base (obs) + lquote.length; errno = 0; - fd = mkstemp ((char *) obstack_base (obs)); + fd = mkstemp (name); if (fd < 0) { - m4_warn (errno, me, _("cannot create tempfile `%s'"), name); + m4_warn (errno, me, _("cannot create tempfile `%s'"), pattern); obstack_free (obs, obstack_finish (obs)); } else { close (fd); - /* Undo trailing NUL. */ - /* FIXME - should we be quoting this name, on the tiny chance - that the random name generated matches a user's macro? */ + /* Remove NUL, then finish quote. */ obstack_blank (obs, -1); + obstack_grow (obs, rquote.string, rquote.length); } } hooks/post-receive -- GNU M4 source repository
