Ralf Wildenhues <[EMAIL PROTECTED]> wrote: > Hi Jim, > > * Jim Meyering wrote on Mon, Nov 24, 2008 at 09:02:51PM CET: >> [ along the way I noticed that my gnulib-tool patch is wrong, >> since single quotes in the context of a Makefile dependency >> list are interpreted literally ] > > Indeed. > >> But I did find a bug to fix... in automake. > >> While $(top_srcdir) values usually look like ../src, >> but sometimes they are full, absolute names. It's the latter >> case I was trying to protect against with the gnulib-tool patch. >> >> For the record, you can cause trouble by building with a >> source directory name containing e.g., a space, and invoking >> configure via an absolute name. > > Been there before, tried to push the exact patch you propose. > It doesn't help but cover up. If the corresponding code complains, > the user used a path he shouldn't have used. "Don't do that when > it hurts." > > It would be an improvement if the sanity.m4 code produced a more > helpful error, though.
Hi Ralf, 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. To demonstrate (as non-root), create a directory named 'a;$(halt);' cd into it, unpack an automake/autoconf-using tarball, then run "$PWD/configure". When I just did that with coreutils.tar.gz, I got this: $ "$PWD/configure" checking for a BSD-compatible install... /p/bin/install -c checking whether build environment is sane... yes /bin/sh: /t/am-test/a: No such file or directory halt: Need to be root /t/am-test/a;$(halt);/coreutils-7.0.76-cb90a/configure: line 3275: /coreutils-7.0.76-cb90a/build-aux/missing: No such file or directory configure: WARNING: `missing' script is too old or missing checking for a thread-safe mkdir -p... /p/bin/mkdir -p ... configure: creating ./config.status halt: Need to be root ... config.status: creating Makefile config.status: creating doc/Makefile config.status: creating po/Makefile $ To my surprise, I found that it invokes halt even when invoked via the usual "./configure": $ ./configure checking for a BSD-compatible install... /p/bin/install -c checking whether build environment is sane... yes /bin/sh: /t/am-test/a: No such file or directory halt: Need to be root ./configure: line 3275: /coreutils-7.0.70-186b6/build-aux/missing: No such file or directory configure: WARNING: `missing' script is too old or missing checking for a thread-safe mkdir -p... /p/bin/mkdir -p That's due to the eval of "$MISSING...". And that's only in the relative safety of the configure script. I'd expect many more problems in arbitrary Makefile.am files. This is why I care about unprotected uses of $(top_srcdir) Here's another patch, just for discussion: [ hmm. just noticed that the autoconf-set $as_cr_* variables are not documented, so we'd have to spell them out. Actually, you might argue that this sanity check belongs in autoconf itself, in which case it _could_ use them. Also, I'm not confident that the list of acceptable bytes is right. ] >From 3888a6645ca10093ca26373c9d22b49181f5bed3 Mon Sep 17 00:00:00 2001 From: Jim Meyering <[EMAIL PROTECTED]> Date: Tue, 25 Nov 2008 10:53:02 +0100 Subject: [PATCH] sanity.m4: disallow risky $PWD * m4/sanity.m4 (AM_SANITY_CHECK): Abort configure if `pwd` contains whitespace or a shell meta-character. --- m4/sanity.m4 | 12 ++++++++++-- 1 files changed, 10 insertions(+), 2 deletions(-) diff --git a/m4/sanity.m4 b/m4/sanity.m4 index 2e76b10..e75b08d 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,6 +16,14 @@ AC_DEFUN([AM_SANITY_CHECK], # Just in case sleep 1 echo timestamp > conftest.file + +# If the absolute working directory name contains any byte that +# would be interpreted as a non-literal by the shell, reject it. +case `pwd` in + [^$as_cr_letters$as_cr_LETTERS$as_cr_digits._,+:/@%=-]) + AC_MSG_ERROR([unsafe absolute working directory name]);; +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 -- 1.6.0.4.1044.g77718
