GNU bug report logs - #28317
Non-portable sh script - $RANDOM

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Neven Sajko <nsajko <at> gmail.com>
Subject: bug#28317: closed (Re: bug#28317: Non-portable sh script - $RANDOM)
Date: Fri, 01 Sep 2017 17:14:03 +0000

[Message part 1 (text/plain, inline)]

Your bug report

#28317: Non-portable sh script - $RANDOM

which was filed against the automake package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 28317 <at> debbugs.gnu.org.

-- 
28317: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=28317
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Eric Blake <eblake <at> redhat.com>
To: Neven Sajko <nsajko <at> gmail.com>, 28317-done <at> debbugs.gnu.org
Subject: Re: bug#28317: Non-portable sh script - $RANDOM
Date: Fri, 1 Sep 2017 12:13:39 -0500

[Message part 3 (text/plain, inline)]

tag 28317 notabug
thanks

On 09/01/2017 07:17 AM, Neven Sajko wrote:
> automake version 1.15.1, also in latest git master.
> 
> See
> 
> https://git.savannah.gnu.org/cgit/automake.git/tree/lib/install-sh#n327

Let's look at it in 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" || {
                   # 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"`
                   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"`
                     test "$ls_ld_tmpdir" = "$ls_ld_tmpdir_1"
                   }
                 }
              then posix_mkdir=:
              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;;

> 
> The RANDOM variable giving pseudo-random numbers is not a POSIX sh
> feature. Dash, for example, does not implement it.

Correct. But for shells that do not implement it, it will expand to the
empty string, at which point we are merely naming our directory ins-$$,
which is still a (somewhat) random name, because it depends on the pid.
True, when $RANDOM is not supported, it's easier to guess the name being
used, but the REAL test is whether the code correctly handles the case
where an attacker races with your probe of an available name and your
subsequent use of the name.  _This_ code is specifically calling mkdir
(which is race-free) as the only use of $tmpdir, and therefore, even
when $RANDOM is not supported, we are not opening ourselves to attack.

Therefore, even though we know it is not POSIX, we also don't care.
This is not a shortcoming that needs to be patched.

> Maybe this would work instead:
> 
> random=`dd 'if=/dev/urandom' 'count=1' 'bs=256' 2>/dev/null | cksum | sed "$r"`\
> `date -u | cksum | sed "$r"`

No, there's no need to furrther complicate something that is already
correct even when $RANDOM is empty.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[signature.asc (application/pgp-signature, attachment)]

[Message part 5 (message/rfc822, inline)]

From: Neven Sajko <nsajko <at> gmail.com>
To: bug-automake <at> gnu.org
Subject: Non-portable sh script - $RANDOM
Date: Fri, 1 Sep 2017 14:17:38 +0200

automake version 1.15.1, also in latest git master.

See

https://git.savannah.gnu.org/cgit/automake.git/tree/lib/install-sh#n327

The RANDOM variable giving pseudo-random numbers is not a POSIX sh
feature. Dash, for example, does not implement it.

So line 327 is probably wrong.

Maybe this would work instead:

random=`dd 'if=/dev/urandom' 'count=1' 'bs=256' 2>/dev/null | cksum | sed "$r"`\
`date -u | cksum | sed "$r"`

If this fix is correct, maybe you would also like to update
https://git.savannah.gnu.org/cgit/automake.git/tree/lib/config.guess#n104


Regards

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.