Package: firehol
Version: 1.231-3
Priority: minor
Tags: patch

While doing a review of usage of tempfiles in Debian I've found out that
the firehol-wizard script uses temporary files in an unsafe
way which could be used to conduct symlink attacks against any user
running it. Notice that this is _not_ the same issue as the one fixed
when #291680 was closed (CAN-2005-0225)

If you review the script you will see that the script will define
FIREHOL_DIR and will then try to call FIREHOL_LIB (which redefines it,
after the fix for CAN-2005-0225 was applied). However if FIREHOL_LIB
cannot be executed for any reason (bad package installation or
the script is run from sources) the code will not abort if it cannot
create sucessfully the temporary directory (which might happen if the 
directory already exists):

      7 FIREHOL_DIR="/tmp/firehol-tmp-$$"
      [ ...] 
      14 test -x $FIREHOL_LIB && . $FIREHOL_LIB
      [ ...] 
      298 #Prepare the directory:
      299 mkdir "${FIREHOL_DIR}"
      300 cd "${FIREHOL_DIR}"

Since the script is not being executed with -e, it will happily
proceeded if a) FIREHOL_LIB is not executable for some reason and
b) the directory is there, so a rogue user could create the
directory and then plant symlinks in files that will be written on
later on (like 'services' or 'unknown_ports').

This is a minor issue, since most package installations will have
$FIREHOL_LIB properly and calling that library the value of
FIREHOL_DIR will be overriden.

In any case, attached is a patch that:

a) fixes this issue by introducing the 
use of mktemp if FIREHOL_LIB does not define FIREHOL_DIR properly. 
It will fail back to the old construct if mktemp is not
present, however. But will take steps to try to create a proper temporary
directory.

b) Clean up FIREHOL_DIR after the script is run (through a trap). Notice
that the firehol-wizard script will _not_ clean up the temporary directory

Again, this is a minor issue, probably cosmetic. But I would appreciate
if it would be fixed so these script does not jump out when
doing source code reviews for /tmp vulnerabilities. 

Regards

Javier
--- firehol-wizard.sh.orig      2005-08-06 17:50:48.000000000 +0200
+++ firehol-wizard.sh   2005-08-23 00:20:40.000000000 +0200
@@ -4,7 +4,6 @@
 # - Sharing rule() (and some other helper functions) between the files
 # - ip_in_net: Do a real check (converting to binary ip)
 
-FIREHOL_DIR="/tmp/firehol-tmp-$$"
 FIREHOL_LIB="/lib/firehol/firehol"
 
 PATH="${PATH}:/bin:/usr/bin:/sbin:/usr/sbin"
@@ -13,6 +12,17 @@
 
 test -x $FIREHOL_LIB && . $FIREHOL_LIB
 
+if [ -z "$FIREHOL_DIR" ] ; then
+       if test -n "`type -p mktemp`" ; then
+               FIREHOL_DIR=`mktemp -d -t firehol.XXXXXX`  || { echo "$program: 
Cannot create temporary dir!" >&2 ; exit 1; }
+       else
+               FIREHOL_DIR=${TMPDIR-/tmp}/firehol.$$
+                       [ -e $FIREHOL_DIR ] && rm -rf $FIREHOL_DIR
+               mkdir -m 0700 $FIREHOLD_DIR || { echo "$0: Cannot create 
temporary dir!" >&2 ; exit 1 ; }
+       fi
+fi
+trap " [ -d \"$FIREHOL_DIR\" ] && /bin/rm -rf -- \"$FIREHOL_DIR\"" 0 1 2 3 13 
15
+
 ##### Helper functions
 
 # require commands for wizard mode
@@ -296,7 +306,6 @@
 
 
 #Prepare the directory:
-mkdir "${FIREHOL_DIR}"
 cd "${FIREHOL_DIR}"
 "${MKDIR_CMD}" ports
 "${MKDIR_CMD}" keys
@@ -683,5 +692,5 @@
        echo "# is not configured for forwarding traffic."
        echo
 fi
-       
+
 exit 0

Attachment: signature.asc
Description: Digital signature

Reply via email to