Re: install-sh patch for compatibility with recent coreutils

[Paul Eggert]

Alexandre Duret-Lutz <address@hidden> writes:

> This looks fine to me.  It would be nice to augment tests/instsh2.test
> with a few lines, though...

Good point, thanks.  Also, some of the existing test cases had to be
changed, as they tested that -c is not the default with 'install'.  I
fixed them too, and installed the following patch into the main branch
of Automake.  Aside from tests/instsh2.test, the only new change here
is to update the date in lib/install-sh.

2004-07-05  Paul Eggert  <address@hidden>

        * doc/automake.texi (Install): Warn that you should create
        /tmp/staging before installing into it, to avoid security problems.
        * lib/install-sh: Remove support for -b= and -t= options; this
        has been moribund for a decade.
        Add support for -t and -T options (new in coreutils install).
        -c option now does nothing (the default is to copy), for
        compatibility with BSD and coreutils 'install'.
        Fix usage message; it referred to nonexistent variables.
        Don't assume 'lasterr' is unset in environment.
        * tests/instsh2.test: Don't assume that install-sh without -c
        moves (it now copies).  Add tests for new -t and -T options.

Index: doc/automake.texi
===================================================================
RCS file: /cvs/automake/automake/doc/automake.texi,v
retrieving revision 1.38
diff -p -u -r1.38 automake.texi
--- doc/automake.texi   16 Jun 2004 19:17:03 -0000      1.38
+++ doc/automake.texi   5 Jul 2004 07:04:27 -0000
@@ -5258,10 +5258,13 @@ path is prefixed with the value of @samp
 into the install area.  Here is an example of typical DESTDIR usage:
 
 @example
+mkdir /tmp/staging &&
 make DESTDIR=/tmp/staging install
 @end example
 
-This places install objects in a directory tree built under
+The @command{mkdir} command avoids a security problem if the attacker
+creates a symbolic link from @file{/tmp/staging} to a victim area;
+then @command{make} places install objects in a directory tree built under
 @file{/tmp/staging}.  If @file{/gnu/bin/foo} and
 @file{/gnu/share/aclocal/foo.m4} are to be installed, the above command
 would install @file{/tmp/staging/gnu/bin/foo} and
Index: lib/install-sh
===================================================================
RCS file: /cvs/automake/automake/lib/install-sh,v
retrieving revision 1.18
diff -p -u -r1.18 install-sh
--- lib/install-sh      2 Apr 2004 01:43:29 -0000       1.18
+++ lib/install-sh      5 Jul 2004 07:04:27 -0000
@@ -1,7 +1,7 @@
 #!/bin/sh
 # install - install a program, script, or datafile
 
-scriptversion=2004-04-01.17
+scriptversion=2004-07-05.00
 
 # This originates from X11R5 (mit/util/scripts/install.sh), which was
 # later released in X11R6 (xc/config/util/install.sh) with the
@@ -58,9 +58,6 @@ stripprog="${STRIPPROG-strip}"
 rmprog="${RMPROG-rm}"
 mkdirprog="${MKDIRPROG-mkdir}"
 
-transformbasename=
-transform_arg=
-instcmd="$mvprog"
 chmodcmd="$chmodprog 0755"
 chowncmd=
 chgrpcmd=
@@ -70,23 +67,27 @@ mvcmd="$mvprog"
 src=
 dst=
 dir_arg=
+dstarg=
+no_target_directory=
 
-usage="Usage: $0 [OPTION]... SRCFILE DSTFILE
+usage="Usage: $0 [OPTION]... [-T] SRCFILE DSTFILE
    or: $0 [OPTION]... SRCFILES... DIRECTORY
-   or: $0 -d DIRECTORIES...
+   or: $0 [OPTION]... -t DIRECTORY SRCFILES...
+   or: $0 [OPTION]... -d DIRECTORIES...
 
-In the first form, install SRCFILE to DSTFILE, removing SRCFILE by default.
-In the second, create the directory path DIR.
+In the 1st form, copy SRCFILE to DSTFILE.
+In the 2nd and 3rd, copy all SRCFILES to DIRECTORY.
+In the 4th, create DIRECTORIES.
 
 Options:
--b=TRANSFORMBASENAME
--c         copy source (using $cpprog) instead of moving (using $mvprog).
+-c         (ignored)
 -d         create directories instead of installing files.
--g GROUP   $chgrp installed files to GROUP.
--m MODE    $chmod installed files to MODE.
--o USER    $chown installed files to USER.
--s         strip installed files (using $stripprog).
--t=TRANSFORM
+-g GROUP   $chgrpprog installed files to GROUP.
+-m MODE    $chmodprog installed files to MODE.
+-o USER    $chownprog installed files to USER.
+-s         $stripprog installed files.
+-t DIRECTORY  install into DIRECTORY.
+-T         report an error if DSTFILE is a directory.
 --help     display this help and exit.
 --version  display version info and exit.
 
@@ -96,12 +97,7 @@ Environment variables override the defau
 
 while test -n "$1"; do
   case $1 in
-    -b=*) transformbasename=`echo $1 | sed 's/-b=//'`
-        shift
-        continue;;
-
-    -c) instcmd=$cpprog
-        shift
+    -c) shift
         continue;;
 
     -d) dir_arg=true
@@ -129,14 +125,20 @@ while test -n "$1"; do
         shift
         continue;;
 
-    -t=*) transformarg=`echo $1 | sed 's/-t=//'`
-        shift
-        continue;;
+    -t) dstarg=$2
+       shift
+       shift
+       continue;;
+
+    -T) no_target_directory=true
+       shift
+       continue;;
 
     --version) echo "$0 $scriptversion"; exit 0;;
 
     *)  # When -d is used, all remaining arguments are directories to create.
-       test -n "$dir_arg" && break
+       # When -t is used, the destination is already specified.
+       test -n "$dir_arg$dstarg" && break
         # Otherwise, the last argument is the destination.  Remove it from 
address@hidden
        for arg
        do
@@ -174,13 +176,13 @@ do
     src=
 
     if test -d "$dst"; then
-      instcmd=:
+      mkdircmd=:
       chmodcmd=
     else
-      instcmd=$mkdirprog
+      mkdircmd=$mkdirprog
     fi
   else
-    # Waiting for this to be detected by the "$instcmd $src $dsttmp" command
+    # Waiting for this to be detected by the "$cpprog $src $dsttmp" command
     # might cause directories to be created, which would be especially bad
     # if $src (and thus $dsttmp) contains '*'.
     if test ! -f "$src" && test ! -d "$src"; then
@@ -202,6 +204,10 @@ do
     # If destination is a directory, append the input filename; won't work
     # if double slashes aren't ignored.
     if test -d "$dst"; then
+      if test -n "$no_target_directory"; then
+       echo "$0: $dstarg: Is a directory" >&2
+       exit 1
+      fi
       dst=$dst/`basename "$src"`
     fi
   fi
@@ -229,34 +235,25 @@ do
       pathcomp=$pathcomp$1
       shift
       if test ! -d "$pathcomp"; then
-        $mkdirprog "$pathcomp" || lasterr=$?
+        $mkdirprog "$pathcomp"
        # mkdir can fail with a `File exist' error in case several
        # install-sh are creating the directory concurrently.  This
        # is OK.
-       test ! -d "$pathcomp" && { (exit ${lasterr-1}); exit; }
+       test -d "$pathcomp" || exit
       fi
       pathcomp=$pathcomp/
     done
   fi
 
   if test -n "$dir_arg"; then
-    $doit $instcmd "$dst" \
+    $doit $mkdircmd "$dst" \
       && { test -z "$chowncmd" || $doit $chowncmd "$dst"; } \
       && { test -z "$chgrpcmd" || $doit $chgrpcmd "$dst"; } \
       && { test -z "$stripcmd" || $doit $stripcmd "$dst"; } \
       && { test -z "$chmodcmd" || $doit $chmodcmd "$dst"; }
 
   else
-    # If we're going to rename the final executable, determine the name now.
-    if test -z "$transformarg"; then
-      dstfile=`basename "$dst"`
-    else
-      dstfile=`basename "$dst" $transformbasename \
-               | sed $transformarg`$transformbasename
-    fi
-
-    # don't allow the sed command to completely eliminate the filename.
-    test -z "$dstfile" && dstfile=`basename "$dst"`
+    dstfile=`basename "$dst"`
 
     # Make a couple of temp file names in the proper directory.
     dsttmp=$dstdir/_inst.$$_
@@ -266,14 +263,14 @@ do
     trap 'status=$?; rm -f "$dsttmp" "$rmtmp" && exit $status' 0
     trap '(exit $?); exit' 1 2 13 15
 
-    # Move or copy the file name to the temp name
-    $doit $instcmd "$src" "$dsttmp" &&
+    # Copy the file name to the temp name.
+    $doit $cpprog "$src" "$dsttmp" &&
 
     # and set any options; do chmod last to preserve setuid bits.
     #
     # If any of these fail, we abort the whole thing.  If we want to
     # ignore errors from any of these, just make sure not to ignore
-    # errors from the above "$doit $instcmd $src $dsttmp" command.
+    # errors from the above "$doit $cpprog $src $dsttmp" command.
     #
     { test -z "$chowncmd" || $doit $chowncmd "$dsttmp"; } \
       && { test -z "$chgrpcmd" || $doit $chgrpcmd "$dsttmp"; } \
Index: tests/instsh2.test
===================================================================
RCS file: /cvs/automake/automake/tests/instsh2.test,v
retrieving revision 1.3
diff -p -u -r1.3 instsh2.test
--- tests/instsh2.test  8 Jan 2004 23:32:11 -0000       1.3
+++ tests/instsh2.test  5 Jul 2004 07:04:27 -0000
@@ -48,7 +48,7 @@ test -d d3
 test -f x
 test -f y
 ./install-sh -m 644 y z
-test ! -f y
+test -f y
 test -f z
 # Multiple files
 ./install-sh -m 644 -c x z d1
@@ -57,14 +57,23 @@ test -f z
 test -f d1/x
 test -f d1/z
 ./install-sh -m 644 x z d2
-test ! -f x
-test ! -f z
+test -f x
+test -f z
 test -f d2/x
 test -f d2/z
+./install-sh -t d3 -m 644 x z
+test -f x
+test -f z
+test -f d3/x
+test -f d3/z
+./install-sh -T x d3/y
+test -f x
+test -f d3/y
+./install-sh -T x d3 && exit 1
 
 # Ensure that install-sh works with names that include spaces
 touch 'a b'
 mkdir 'x y'
 ./install-sh 'a b' 'x y'
 test -f x\ y/a\ b
-test ! -f 'a b'
+test -f 'a b'