[m-dev.] for review: fix security holes.
Fergus Henderson
fjh at cs.mu.OZ.AU
Thu Mar 26 10:56:25 AEDT 1998
> > Better not use $0 there, it might contain /'s.
> > I think it would be simplest to just hard-code `ml'.
>
> Why would you want to hardcode `ml'?
>
> This is the configure script that is running -- hardcoding `configure'
> might be better.
Sorry, asleep at the wheel here. If it is `configure' that is creating
the file, then yes it makes sense to hardcode `configure', not `ml'.
This is perhaps not ideal, since the name `configure' is not very
unique, but that shouldn't be a problem, since mktemp is supposed
to be able to deal with such things.
Note that the normal solution when using autoconf would be to name
the temporary files `conftest*', as described in the following
quote from the autoconf documentation:
If a test program needs to use or create a data file, give it a name
that starts with `conftest', such as `conftestdata'. The `configure'
script cleans up by running `rm -rf conftest*' after running test
programs and if the script is interrupted.
However, since the real program is going to create a temp file in /tmp,
it is probably better to have the autoconf test do the same
(lest it work in one case but not the other).
> +MERCURY_MSG("looking for a way to create temporary files...")
> +
> +AC_PATH_PROG(MKTEMP,mktemp)
> +if test "$MKTEMP" != ""; then
> + # check that it really works
> + TMPFILE=`mktemp /tmp/configure.XXXXXX`
It might be better to make that
TMPFILE=`mktemp /tmp/configure.XXXXXX` || exit 1
> + if [ $MKTEMP = "" ] ; then
On many systems it is slightly more efficient to write this as a
`case' statement
case $MKTEMP in
"") ... ;;
*) ... ;;
esac
rather than using `if [ ... ]'. The reason for this is that
often `[' is not a shell builtin. (E.g. this is true if you
use `ash' rather than `bash' as /bin/sh.)
> + old_umask=`umask`
> + umask 022
> + mmake_tmpdir=/tmp/mmake$$
> + tmp=$mmake_tmpdir/mmake
> + if ! mkdir $mmake_tmpdir ; then
> + echo "Unable to create temporary makefile" 1>&2
> + exit 1
> + fi
> + umask $old_umask
> + trap 'status=$?; rm -rf $mmake_tmpdir; exit $status' 0 1 2 3 13 15
It would be slightly better to swap the order of the `umask' and `trap'
statements. There is a very small timing window where if someone
hits control-C between the mkdir and the trap, the temp file and directory
will not be removed. I don't see any simple way of fixing that, and it is
a very minor problem, but you should make the timing window as small
as possible.
Otherwise that looks fine, so if you want commit after making those
three changes, please go ahead.
--
Fergus Henderson <fjh at cs.mu.oz.au> | "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh> | of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3 | -- the last words of T. S. Garp.
More information about the developers
mailing list