Automake and whitespace in pwd (was: [PATCH] gnulib-tool: do not use $(top_srcdir) unquoted; may be tainted)

[Ralf Wildenhues]

Hello Jim,

* Jim Meyering wrote on Tue, Nov 25, 2008 at 10:59:36AM CET:
> 
> I think it's a fine idea to disallow bogus (and potentially dangerous)
> absolute names in automake's sanity.m4, but the existing code is not
> sufficient.

Agreed.

> To demonstrate (as non-root), create a directory named 'a;$(halt);'
> cd into it, unpack an automake/autoconf-using tarball, then run
> "$PWD/configure".

OK, let's improve the situation here.  I agree that whitespace in
$srcdir does not work, same with some other special characters.
However, I dislike forbidding whitespace in `pwd`, I don't want to
needlessly make Automake less useful for some legitimate uses.  Also, I
prefer to go with what tests/instspc.test lists as broken rather than
explicitly forbidding everything.  Let's give users with whitespace in
`pwd` some slack and make things harder for package authors instead.
;->

What do you think of this patch?

Combined with some trivial fixes to texi2dvi, the only test failures I
get with source tree in `/tmp/dir  with  spaces/automake' and build tree
in `/tmp/dir  with  spaces/build' and a $srcdir of ../automake are those
that stem from dejagnu (runtest) failures.  All tests involving gettext
or libtool are skipped, however, AFAIK at least libtool does not cope
with white space in `pwd`.

BTW, choosing to use single-quoting for $install_sh but double-quoting
for $MISSING is ugly, but not arbitrary: `install-strip' already has a
reason to use double quoting for "$(INSTALL_STRIP_PROGRAM)" which may
contain $install_sh.  And $MISSING uses double-quoting so that does not
interfere with the single-quoting used by texi2dvi.  Ugly, yes.
And not quoting when no whitespace is present is of course done to
prevent breaking all those applications that are not prepared for this.
Many packages will certainly not work with whitespace in $PWD, and it
would make little sense to require them to adjust their rules for
changed quoting in $MISSING.

And yes, there are more warts: A compiler without "-c -o" won't work.
Several tests skip because things won't work in a directory name with
white space.

Cheers,
Ralf

2008-11-26  Ralf Wildenhues  <address@hidden>
            Jim Meyering <address@hidden>

        Cope with whitespace in $MISSING and $install_sh.
        * configure.ac (am_AUTOHEADER): New substitution, save the value
        of $AUTOHEADER before AM_INIT_AUTOMAKE may add $MISSING.
        * tests/defs.in: Use am_AUTOHEADER.
        * lib/am/install.am: Fix typo.
        * m4/install-sh.m4 (AM_PROG_INSTALL_SH): Add suitable
        single-quote quoting to install_sh, but only if needed.
        * m4/missing.m4 (AM_MISSING_HAS_RUN): Add suitable double-quote
        quoting to MISSING, but only if needed.
        * m4/sanity.m4 (AM_SANITY_CHECK): Abort configure if `pwd` or
        $srcdir contain shell meta-characters that cannot be handled;
        space and tab are allowed in the former only.
        * tests/sanity.test: New test.
        * tests/Makefile.am: Adjust.
        * NEWS: Update.
        Reports by Jim Meyering and others.

diff --git a/NEWS b/NEWS
index 27f8368..2721e69 100644
--- a/NEWS
+++ b/NEWS
@@ -138,6 +138,13 @@ New in 1.10a:
 
   - The `missing' script works better with versioned tool names.
 
+  - The Automake macros and rules cope better with whitespace in the
+    current directory name, as long as the relative path to `configure'
+    does not contain whitespace.  To this end, the values of `$(MISSING)'
+    and `$(install_sh)' may contain suitable quoting, and their expansion
+    might need `eval'uation if used outside of a makefile.  These
+    undocumented variables may be used in several documented macros.
+
 Bugs fixed in 1.10a:
 
 * Long standing bugs:
diff --git a/configure.ac b/configure.ac
index f049533..592da2e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -25,8 +25,9 @@ AC_CANONICAL_BUILD
 
 # Save the AUTOCONF setting before AM_INIT_AUTOMAKE overrides it; this
 # way we can run Autoconf tests from configure (or from the test
-# suite) without being bothered by `missing'.
+# suite) without being bothered by `missing'.  Likewise for autoheader.
 AC_SUBST([am_AUTOCONF], ["${AUTOCONF-autoconf}"])
+AC_SUBST([am_AUTOHEADER], ["${AUTOHEADER-autoheader}"])
 
 AM_INIT_AUTOMAKE([1.10a dist-bzip2 filename-length-max=99 color-tests 
parallel-tests])
 
diff --git a/lib/am/install.am b/lib/am/install.am
index 885ad0b..c114fa5 100644
--- a/lib/am/install.am
+++ b/lib/am/install.am
@@ -1,5 +1,5 @@
 ## automake - create Makefile.in from Makefile.am
-## Copyright (C) 2001, 2002, 2003, 2004, 2006  Free Software Foundation, Inc.
+## Copyright (C) 2001, 2002, 2003, 2004, 2006, 2008  Free Software Foundation, 
Inc.
 
 ## This program is free software; you can redistribute it and/or modify
 ## it under the terms of the GNU General Public License as published by
@@ -78,7 +78,7 @@ install-am: all-am
 ## directory.
 .MAKE .PHONY: install-strip
 install-strip:
-## Beware that they are two variables used to install programs:
+## Beware that there are two variables used to install programs:
 ##   INSTALL_PROGRAM is used for ordinary *_PROGRAMS
 ##   install_sh_PROGRAM is used for nobase_*_PROGRAMS (because install-sh
 ##                                                     creates directories)
diff --git a/m4/install-sh.m4 b/m4/install-sh.m4
index 32c2ebb..b153715 100644
--- a/m4/install-sh.m4
+++ b/m4/install-sh.m4
@@ -1,5 +1,5 @@
 ##                                                          -*- Autoconf -*-
-# Copyright (C) 2001, 2003, 2005  Free Software Foundation, Inc.
+# Copyright (C) 2001, 2003, 2005, 2008  Free Software Foundation, Inc.
 #
 # This file is free software; the Free Software Foundation
 # gives unlimited permission to copy and/or distribute it,
@@ -10,5 +10,12 @@
 # Define $install_sh.
 AC_DEFUN([AM_PROG_INSTALL_SH],
 [AC_REQUIRE([AM_AUX_DIR_EXPAND])dnl
-install_sh=${install_sh-"\$(SHELL) $am_aux_dir/install-sh"}
+if test x"${install_sh}" != xset; then
+  case $am_aux_dir in
+  *\ * | *\    *)
+    install_sh="\${SHELL} '$am_aux_dir/install-sh'" ;;
+  *)
+    install_sh="\${SHELL} $am_aux_dir/install-sh"
+  esac
+fi
 AC_SUBST(install_sh)])
diff --git a/m4/missing.m4 b/m4/missing.m4
index 26fab2a..136399c 100644
--- a/m4/missing.m4
+++ b/m4/missing.m4
@@ -1,13 +1,13 @@
 # Fake the existence of programs that GNU maintainers use.  -*- Autoconf -*-
 
-# Copyright (C) 1997, 1999, 2000, 2001, 2003, 2004, 2005
+# Copyright (C) 1997, 1999, 2000, 2001, 2003, 2004, 2005, 2008
 # Free Software Foundation, Inc.
 #
 # This file is free software; the Free Software Foundation
 # gives unlimited permission to copy and/or distribute it,
 # with or without modifications, as long as this notice is preserved.
 
-# serial 5
+# serial 6
 
 # AM_MISSING_PROG(NAME, PROGRAM)
 # ------------------------------
@@ -24,7 +24,14 @@ AC_SUBST($1)])
 AC_DEFUN([AM_MISSING_HAS_RUN],
 [AC_REQUIRE([AM_AUX_DIR_EXPAND])dnl
 AC_REQUIRE_AUX_FILE([missing])dnl
-test x"${MISSING+set}" = xset || MISSING="\${SHELL} $am_aux_dir/missing"
+if test x"${MISSING+set}" != xset; then
+  case $am_aux_dir in
+  *\ * | *\    *)
+    MISSING="\${SHELL} \"$am_aux_dir/missing\"" ;;
+  *)
+    MISSING="\${SHELL} $am_aux_dir/missing" ;;
+  esac
+fi
 # Use eval to expand $SHELL
 if eval "$MISSING --run true"; then
   am_missing_run="$MISSING --run "
diff --git a/m4/sanity.m4 b/m4/sanity.m4
index 2e76b10..1704d83 100644
--- a/m4/sanity.m4
+++ b/m4/sanity.m4
@@ -1,13 +1,13 @@
 # Check to make sure that the build environment is sane.    -*- Autoconf -*-
 
-# Copyright (C) 1996, 1997, 2000, 2001, 2003, 2005
+# Copyright (C) 1996, 1997, 2000, 2001, 2003, 2005, 2008
 # Free Software Foundation, Inc.
 #
 # This file is free software; the Free Software Foundation
 # gives unlimited permission to copy and/or distribute it,
 # with or without modifications, as long as this notice is preserved.
 
-# serial 4
+# serial 5
 
 # AM_SANITY_CHECK
 # ---------------
@@ -16,16 +16,29 @@ AC_DEFUN([AM_SANITY_CHECK],
 # Just in case
 sleep 1
 echo timestamp > conftest.file
+# Reject unsafe characters in $srcdir or the absolute working directory
+# name.  Accept space and tab only in the latter.
+am_lf='
+'
+case `pwd` in
+  *[[\"\#\$\&\'\\\`$am_lf]]*)
+    AC_MSG_ERROR([unsafe absolute working directory name]);;
+esac
+case $srcdir in
+  *[[\"\#\$\&\'\\\`$am_lf\ \   ]]*)
+    AC_MSG_ERROR([unsafe srcdir value: `$srcdir']);;
+esac
+
 # Do `set' in a subshell so we don't clobber the current shell's
 # arguments.  Must try -L first in case configure is actually a
 # symlink; some systems play weird games with the mod time of symlinks
 # (eg FreeBSD returns the mod time of the symlink's containing
 # directory).
 if (
-   set X `ls -Lt $srcdir/configure conftest.file 2> /dev/null`
+   set X `ls -Lt "$srcdir/configure" conftest.file 2> /dev/null`
    if test "$[*]" = "X"; then
       # -L didn't work.
-      set X `ls -t $srcdir/configure conftest.file`
+      set X `ls -t "$srcdir/configure" conftest.file`
    fi
    rm -f conftest.file
    if test "$[*]" != "X $srcdir/configure conftest.file" \
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 43f3b8d..dffcb86 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -543,6 +543,7 @@ req.test \
 reqd.test \
 reqd2.test \
 rulepat.test \
+sanity.test \
 scripts.test \
 seenc.test \
 sinclude.test \
diff --git a/tests/defs.in b/tests/defs.in
index cd01e6e..f160e2b 100644
--- a/tests/defs.in
+++ b/tests/defs.in
@@ -61,7 +61,7 @@ export SHELL
 test -z "$PERL" && PERL='@PERL@'
 test -z "$MAKE" && MAKE=make
 test -z "$AUTOCONF" && AUTOCONF="@am_AUTOCONF@"
-test -z "$AUTOHEADER" && AUTOHEADER="@AUTOHEADER@"
+test -z "$AUTOHEADER" && AUTOHEADER="@am_AUTOHEADER@"
 test -z "$AUTOUPDATE" && AUTOUPDATE=autoupdate
 test -z "$MISSING" && MISSING=`pwd`/../lib/missing
 # Use -Werror because this also turns some Perl warnings into error.
diff --git a/tests/sanity.test b/tests/sanity.test
new file mode 100755
index 0000000..0d3830d
--- /dev/null
+++ b/tests/sanity.test
@@ -0,0 +1,49 @@
+#! /bin/sh
+# Copyright (C) 2008 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Unsafe absolute directory names should be diagnosed.
+
+. ./defs || Exit 1
+
+set -e
+
+mkdir 'unsafe$'
+cd 'unsafe$'
+
+cat > configure.in << 'END'
+AC_INIT([sanity], [1.0])
+AM_INIT_AUTOMAKE([foreign])
+AC_OUTPUT(Makefile)
+END
+
+cp ../install-sh ../missing .
+
+: > Makefile.am
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE
+./configure 2>stderr && { cat stderr >&2; Exit 1; }
+cat stderr
+grep 'unsafe absolute working directory' stderr
+
+cd ..
+mkdir build
+cd build
+../unsafe$/configure 2>stderr && { cat stderr >&2; Exit 1; }
+cat stderr
+grep 'unsafe srcdir' stderr
+: