Re: [hylafax-users] WARNING: Potential Risk With Faxsetup

[Aidan Van Dyk <aidan@xxxxxxxx>]

* A. Kalten <akalten@xxxxxxxxxxx> [070103 12:25]:
> While doing some very quick compiling and installing
> from one HylaFax version to another as part of a quick
> testing process, I managed to delete my entire /tmp
> directory which contained a lot of important system
> files.  Fortunately, I had a backup, but since the backup
> was at least 24 hours old, a few files were irrecoverable.
> 
> The culprit was the HylaFAX faxsetup script.  I don't recall
> exactly what happened, but at one point I had exited the
> script using the CTRL-C key.  When faxsetup was run again,
> it completely removed the /tmp directory.

> Faxsetup contains a variable TMPDIR which is used to
> refer to a temporary storage directory.  This is a bad
> choice because a system variable TMPDIR already exists.
> On my system, TMPDIR = /tmp, and somehow, when faxsetup
> was restarted, the value of this variable was retained.
> Because this directory is removed by faxsetup, my entire
> /tmp directory was deleted.

Hm... I don't quicly see how this could happen.

There are 2 places where $TMPDIR is removed:

1) The trap on early exits (line 916 CVS):
	trap "$RM \$JUNK; $RM -r \$TMPDIR; exit 1" 1 2 15
2) The final RM at the end:
	$RM -r $TMPDIR

The reason I don't see this happening is because Neither of those 2
locations can be reached until *after* we explicity set TMPDIR
ourselves:

    # Setup TMPDIR before anything can trap and rm it
    TMPDIR=`(mktemp -d /tmp/.faxsetup.XXXXXX) 2>/dev/null`
    if test x$TMPDIR = x; then
        TMPDIR=/tmp/.faxsetup$$
    fi
    $RM -rf $TMPDIR

I don't see how this can leave TMPDIR set to your system "/tmp", but not
exit.  If you CTRL-C in here, somehow in teh TMPDIR assignment, it exits
(the trap is not yet installed) before getting to any rm.  If the
asignment success or fails, I can't see how TMPDIR could possibly be
/tmp.  Can you find a way to reproduce this, not necessarily using
TMPDIR=/tmp, but similar code that can show how the variable we just set
is some previous value?

> The name of this variable should be changed to TEMPDIR
> or TEMP or something else that will not conflict with
> the already present TMPDIR system variable.

> From now on, I will manually alter this variable name to
> avoid another potential disaster.

The reason we use TMPDIR is because it is the expected variable.  We
*want* any subprocesses/utilities we run to use the private TMPDIR we
just created.  If they don't, then we will leave your original $TMPDIR
littered with files that we currently can clean up.


a.

-- 
Aidan Van Dyk                                             aidan@xxxxxxxx
Senior Software Developer                          +1 215 825-8700 x8103
iFAX Solutions, Inc.                                http://www.ifax.com/

Attachment: signature.asc
Description: Digital signature