[PATCH] install-sh: avoid (low risk) race in /tmp

Pavel Raiskup <[email protected]>
Newsgroups gmane.comp.sysutils.automake.patches
Message-ID <[email protected]>
Ensure that nobody can cross privilege boundaries by pre-creating
symlink on '$tmpdir' path.

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.

Note that 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.
(scriptversion): Bump.
---
 lib/install-sh | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/lib/install-sh b/lib/install-sh
index 0b0fdcb..802fff4 100755
--- a/lib/install-sh
+++ b/lib/install-sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 # install - install a program, script, or datafile
 
-scriptversion=2013-12-25.23; # UTC
+scriptversion=2015-04-29.07; # UTC
 
 # This originates from X11R5 (mit/util/scripts/install.sh), which was
 # later released in X11R6 (xc/config/util/install.sh) with the
@@ -324,34 +324,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;;
-- 
2.1.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.