On Saturday 28 August 2004 05:15, Christian Boltz wrote:
aka "symlink attack", i assume.
Yup... :o)
TDIR=${TMPDIR:-/tmp}/aview_$$
Insecure. $$ is guessable (or, worst case: for i in `seq 2 33000 ; do ln -s /home/victim/Mail/inbox /tmp/aview_$i ; done - no more need to guess ;-)
Use mktemp instead:
TDIR=`mktemp -d /tmp/aview.XXXXXXXXXX` || { echo "unable to create temp dir" >&2; exit 1; }
I avoided using mktemp because the aalib code runs on lots of different platforms. From what I've read, I can't be sure that mktemp is available on all of them. So....:
trap clear 0 (umask 077 && mkdir $TDIR) || { echo "Unable to create temp directory $TDIR" exit 1 } mkfifo $FIFO || { echo "Unable to create FIFO $FIFO" exit 1 }
These blocks are no longer needed because mktemp already creates the temp dir and fifo.
...these blocks are needed! I think that's the platform independent, secure way to create a temporary directory, and if there's a nasty link in place it will fail.
if anytopnm $1 >$FIFO 2>/dev/null ; then
^^ Variables should be quoted: "$1"
Good point! I only worried about the symlink issues (slaps own wrists)!
while true; do echo "0 " done
This is an endless loop just printing "0 " on your screen.
Yeah, weird eh? I decided to only concentrate on the security aspects of the script. Presumably the original author felt good reason to fill the screen with 0s!
Yours,
Christian Boltz
Thanks, I've learnt more useful stuff... :o)