Re: [PATCH] gnulib-tool: do not use $(top_srcdir) unquoted; may be tainted

[Jim Meyering]

Ralf Wildenhues <address@hidden> wrote:
> Hello Jim,
>
> * Jim Meyering wrote on Mon, Nov 24, 2008 at 05:09:08PM CET:
>> I noticed unquoted uses of $(top_srcdir) in lib/Makefile.am
>> and found that gnulib-tool generated them.
>> While that's normally not a problem (most of us use well-behaved names),
>> it can lead to malfunction or even serious abuse with ill-chosen or
>> malicious absolute source directory names.
>
> While I agree that absolute paths should be treated with caution,
> I don't think we can make autotools' and gnulib's outputs really
> safe against arbitrary characters in $(srcdir) and $(top_srcdir),
> which are usually relative.  For a simple example, `make' prevents
> many characters to work reliably in these paths.
>
> git Automake and Autoconf are a lot safer for $(abs_*) paths than
> they were before, but not for relative paths.  So if `pwd` contains
> ugliness, we expect from the user not to invoke configure with an
> absolute path (Autoconf rationalizes `pwd`/configure, though).

Hi Ralf,

[ 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 ]

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.

I've just tried it, and found that automake's own sanity.m4
makes configure fail in that case, incorrectly accusing ls -t
of not working properly.  With the following patch, a regenerated
configure.in gets past the sanity test.


>From 4d3f1273844b89bcc5ff5efa4759f451a586543a Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 24 Nov 2008 20:56:37 +0100
Subject: [PATCH] sanity.m4: don't falsely accuse "ls -t" of failure

* m4/sanity.m4 (AM_SANITY_CHECK): Work also when $srcdir
contains a space or shell meta-character.  This test would fail
unnecessarily only when configure is invoked via an absolute name
containing the offending bytes.
---
 m4/sanity.m4 |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/m4/sanity.m4 b/m4/sanity.m4
index 2e76b10..5488ef0 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
 # ---------------
@@ -22,10 +22,10 @@ echo timestamp > conftest.file
 # (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" \
--
1.6.0.4.1044.g77718