Re: install-sh and $RANDOM

[Eric Blake]

On 10/17/2016 03:58 PM, address@hidden wrote:
> Hi,
> 
> in build-aux/install-sh scriptversion=2016-01-11.22 line 327
> tmpdir=${TMPDIR-/tmp}/ins$RANDOM-$$
> ... things which are spuriously bad if $RANDOM does not exist ...

Thanks for the report.  However, you've made two mistakes.

First, gnulib does not maintain install-sh - that is automake's job, so
if anything needs to change, it would have to change upstream in
automake first.

Second, your claim that things are "spuriously bad if $RANDOM does not
exist" is false.  Look at the full context:

            tmpdir=${TMPDIR-/tmp}/ins$RANDOM-$$
            trap 'ret=$?; rmdir "$tmpdir/d" "$tmpdir" 2>/dev/null; exit
$ret' 0

            if (umask $mkdir_umask &&
                exec $mkdirprog $mkdir_mode -p -- "$tmpdir/d")
>/dev/null 2>&1
            then
              if test -z "$dir_arg" || {
...
              fi
              rmdir "$tmpdir/d" "$tmpdir"
            else
              # Remove any dirs left behind by ancient mkdir
implementations.
              rmdir ./$mkdir_mode ./-p ./-- 2>/dev/null
            fi
            trap '' 0;;
        esac;;
    esac

    if
      $posix_mkdir && (
        umask $mkdir_umask &&
        $doit_exec $mkdirprog $mkdir_mode -p -- "$dstdir"
      )
    then :
    else

      # The umask is ridiculous, or mkdir does not conform to POSIX,
      # or it failed possibly due to a race condition.  Create the
      # directory the slow way, step by step, checking for races as we go.

Whether the directory is named '/tmp/ins1234-5678' (on bash) or
'/tmp/ins-5678' (on shells that lack $RANDOM), the remaining code is
STILL atomically correct - either the mkdir succeeds or fails, but there
is NO way that the script can be coerced into mistakenly acting on an
unintended file because someone was able to predict the directory name.
True, collisions are more likely on a setup without $RANDOM, but the
result of a collision is merely that the script gracefully falls back to
slower code, NOT that it operates unsafely on the colliding name.

You're not the first person to complain that $RANDOM is a bashism, and
this is not the first time we've had to retort that our use of $RANDOM
is a nicety, but not a necessity, and that the code is perfectly safe
and tested on shells where the expansion of $RANDOM is the empty string.

> 
> Please use something like:
> tmpdir=$(mktemp -d -p ${TMPDIR:-/tmp})
> 
> Note also :
> 
> NetBSD /bin/sh does not have $RANDOM.
> I don't know how portable mktemp is. sorry.

Less portable than $RANDOM expanding to the empty string.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature