]> git.ipfire.org Git - thirdparty/automake.git/commitdiff
install-sh: avoid (low risk) race in "/tmp"
authorPavel Raiskup <praiskup@redhat.com>
Sun, 11 Mar 2018 20:47:54 +0000 (21:47 +0100)
committerMathieu Lirzin <mthl@gnu.org>
Sun, 11 Mar 2018 21:18:51 +0000 (22:18 +0100)
Ensure that nobody can cross privilege boundaries by pre-creating
symlink on '$tmpdir' destination directory.

Just testing 'mkdir -p' by creating "/tmp/ins$RANDOM-$$/d" is not safe
because "/tmp" directory is usually world-writeable and
"/tmp/ins$RANDOM-$$" content could be pretty easily guessed by
attacker (at least for shells where $RANDOM is not supported).  So, as
the first step, create the "/tmp/ins$RANDOM-$$" without -p.  This step
would fail early if somebody wanted catch us.

Systems that implement (and have enabled) fs.protected_symlinks kernel
feature are not affected even without this commit.

References:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=760455
https://bugzilla.redhat.com/show_bug.cgi?id=1140725

* lib/install-sh: Implement safer 'mkdir -p' test by running
'$mkdirprog $mkdir_mode "$tmpdir"' first.
* NEWS: Update.

Signed-off-by: Mathieu Lirzin <mthl@gnu.org>
NEWS
lib/install-sh

diff --git a/NEWS b/NEWS
index ae26be3bd0afcb79cb8fefa0492f29701d706583..32efd80f29a5855f86777e7c543b8961eeb0e3c1 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -66,6 +66,9 @@ New in ?.?.?:
 
 * Bugs fixed:
 
+  - 'install-sh' now ensures that nobody can cross privilege boundaries by
+    pre-creating symlink on the directory inside "/tmp".
+
   - 'automake' does not depend on the 'none' subroutine of the List::Util
     module anymore to support older Perl version. (automake bug#30631)
 
index 5f3d36cb761fc1d2527551455e4b0775a5ce0616..8175c640fe6288a75cc846567ea5506086f326f4 100755 (executable)
@@ -1,7 +1,7 @@
 #!/bin/sh
 # install - install a program, script, or datafile
 
-scriptversion=2018-03-07.03; # UTC
+scriptversion=2018-03-11.20; # UTC
 
 # This originates from X11R5 (mit/util/scripts/install.sh), which was
 # later released in X11R6 (xc/config/util/install.sh) with the
@@ -332,34 +332,43 @@ do
             # is incompatible with FreeBSD 'install' when (umask & 300) != 0.
             ;;
           *)
+            # Note that $RANDOM variable is not portable (e.g. dash);  Use it
+            # here however when possible just to lower collision chance.
             tmpdir=${TMPDIR-/tmp}/ins$RANDOM-$$
-            trap 'ret=$?; rmdir "$tmpdir/d" "$tmpdir" 2>/dev/null; exit $ret' 0
 
+            trap 'ret=$?; rmdir "$tmpdir/a/b" "$tmpdir/a" "$tmpdir" 2>/dev/null; exit $ret' 0
+
+            # Because "mkdir -p" follows existing symlinks and we likely work
+            # directly in world-writeable /tmp, make sure that the '$tmpdir'
+            # directory is successfully created first before we actually test
+            # 'mkdir -p' feature.
             if (umask $mkdir_umask &&
-                exec $mkdirprog $mkdir_mode -p -- "$tmpdir/d") >/dev/null 2>&1
+                $mkdirprog $mkdir_mode "$tmpdir" &&
+                exec $mkdirprog $mkdir_mode -p -- "$tmpdir/a/b") >/dev/null 2>&1
             then
               if test -z "$dir_arg" || {
                    # Check for POSIX incompatibilities with -m.
                    # HP-UX 11.23 and IRIX 6.5 mkdir -m -p sets group- or
                    # other-writable bit of parent directory when it shouldn't.
                    # FreeBSD 6.1 mkdir -m -p sets mode of existing directory.
-                   ls_ld_tmpdir=`ls -ld "$tmpdir"`
+                   test_tmpdir="$tmpdir/a"
+                   ls_ld_tmpdir=`ls -ld "$test_tmpdir"`
                    case $ls_ld_tmpdir in
                      d????-?r-*) different_mode=700;;
                      d????-?--*) different_mode=755;;
                      *) false;;
                    esac &&
-                   $mkdirprog -m$different_mode -p -- "$tmpdir" && {
-                     ls_ld_tmpdir_1=`ls -ld "$tmpdir"`
+                   $mkdirprog -m$different_mode -p -- "$test_tmpdir" && {
+                     ls_ld_tmpdir_1=`ls -ld "$test_tmpdir"`
                      test "$ls_ld_tmpdir" = "$ls_ld_tmpdir_1"
                    }
                  }
               then posix_mkdir=:
               fi
-              rmdir "$tmpdir/d" "$tmpdir"
+              rmdir "$tmpdir/a/b" "$tmpdir/a" "$tmpdir"
             else
               # Remove any dirs left behind by ancient mkdir implementations.
-              rmdir ./$mkdir_mode ./-p ./-- 2>/dev/null
+              rmdir ./$mkdir_mode ./-p ./-- "$tmpdir" 2>/dev/null
             fi
             trap '' 0;;
         esac;;