Package: hylafax
Severity: critical
Version: 1:4.2.1-7
Tags: woody sarge security patch

This was a mail sent to the Debian security team, the hylafax Debian
maintainer and the hylafax upstream maintainer a while back, since
then, version  1:4.2.2+rc1 has been uploaded to testing (Sept 3rd 2005)
making this issue public:

   * Added patches from Javier Fernandez-Sanguino Peña to cron jobs in
     order to redirect stderr, and to other scripts in order to fix
     temporary directory usage. (See patch 702)

This is the mail sent describing the vulnerabilites (dated 6 Aug 2005).
Attached is the patch (700, not 702 as mentioned in the changelog) used by
the maintainer and based in may original patches.

Notice that the patch only fixes the tempdir vulnerabilities, the last issue
(the UNIX domain socket) has not yet been investigated.

-------------------------------------------------------------------------


Hi there hylafax maintainer and Debian security team,

While reviewing Debian packages for vulnerabilities due to the insecure
use of temporary files I've found that the hylafax package contains a 
script (xferfaxstats) which is vulnerable to symlink attacks since
it uses temporary files in an unsafe way:

------------------------------------------------------------------
(...)
tmpAwk=/tmp/xferfax$$
trap "rm -f $tmpAwk; exit 1" 0 1 2 15
(...)
)>$tmpAwk
$AWK -f $tmpAwk -v TODAY="$TODAY" -v AGE="$AGE" -v SINCEDT="$SINCEDT" -v ENDDT="
------------------------------------------------------------------

Furthermore, this script is run in a monthly basis by the predefined cron
tasks in the package as root so it makes this vulnerability an exploitable
issue in all systems that have this package installed.

I've reviewed all other hylafax scripts which make use of /tmp too:
/usr/sbin/recvstats, /usr/sbin/faxcron, /usr/sbin/faxaddmodem, 
/usr/sbin/faxsetup and /usr/sbin/probemodem and they use constructs which
prevent symlink attacks from overwritting files. It seems that the Debian
maintener fixed these bugs in 28 Sep 1998 (based on the changelog) but
the xferstats script seems to have been missed. 

This script is available in all hylafax-server versions: woody, sarge,
etch and sid.

I'm also concerned that the hylafax package creates /var/spool/hylafax/tmp
with mode 4777 supposedly, based on the manpages of those scripts, to be
used as a temporary location of files but, instead, all of the files use
/tmp instead directly.

In order to clean up the code a little bit attached is a separate
(untested) patch which reviews all the use of temporary files in scripts:

1.- It makes all of them use mktemp if available, this prevents DoS attacks
    agains the scripts since rogue users will find it more difficult
    to pre-create the temporary files. With the current code it is
    simple to prevent all scripts from executing just by populating
    the /tmp directory with symlinks.
2.- It makes all of them use TMPDIR if defined (through mktemp -t or
    through the use of $TMPDIR instead of /tmp if mktemp is not available)
3.- It generates proper error messages if temporary files cannot be
    created
4.- It changes cron jobs to forward error messages to the scripts to log
    files or to mail messages
5.- Do not define 'exit 1' when setting traps, since the exit status 
    of the script will be changed (a 0 exit status will be 1 instead).
    Code sample:
        ------------------------------------------------
        #!/bin/sh

        trap "echo trap; exit 1" 0 1 2 3 15
        echo no trap

        exit 0
        ----------------------------------------------
        $ ./test.sh
        no trap
        trap
        $ echo $?
        1

This patch could be used in the sid package since it is actually code
cleamup. If it was added, the cron jobs could redefine TMPDIR to be 
/var/spool/hylafax/tmp before calling the scripts so that the claim 
of the manpages would be correct. Based on the current code that directory
does not seem to be used at all.

Finally, another concern, is that the default config file defines
FAX_DEFUNIX as /tmp/hyla.unix to setup a transport mode based on UNIX
domain sockets. The FaxClient code uses whatever transport mechanism
is available (through Transport::getTransport) and the Transport code says:

[ ./util/Transport.c++ ]
     53         if (UnixTransport::isA(FAX_DEFUNIX)) {
     54             client.setHost(FAX_DEFUNIX);
     55             return *new UnixTransport(client);
     56         } else {
     57             client.setHost(FAX_DEFHOST);
     58             return *new InetTransport(client);
     59         }

UnixTransport::isA  is defined as:

[ util/UnixTransport.c++ ]
     35 UnixTransport::isA(const char* address)
     36 {
     37      return Sys::isSocketFile(address);
     38 }

I don't see that the Debian package creates the UNIX domain socket at all.
¿Does this code means that if a rogue local user where to create a UNIX
socket at /tmp/hyla.unix all Hylafax clients in the system would use that
instead of the other inettransport (localhost:4559)? ¿Does this means
that a local user could get access to all faxes sent and prevent faxes
from being sent to the proper Hylafax Server? Notice that this code
disputes the claim from the hylafax-client that it only communicates through
TCP/IP (the manpage does not mention it either) and can introduce
a different vulnerability in the system than the ones fixed by the
attached patches.

Regards

Javier Fernandez-Sanguino


#!/bin/sh -e
## 700_hylafax-tmpdir by Giuseppe Sacco <[EMAIL PROTECTED]>
##
## All lines beginning with `## DP:' are a description of the patch.
## DP: This patch fixes a lot of problems with temporary directory
## DP: It was written by Javier Fernández-Sanguino Peña

if [ $# -lt 1 ]; then
    echo >&2 "`basename $0`: script expects -patch|-unpatch as argument"
    exit 1
fi

[ -f debian/patches/00patch-opts ] && . debian/patches/00patch-opts
patch_opts="${patch_opts:--f --no-backup-if-mismatch} ${2:+-d $2}"

case "$1" in
       -patch) patch $patch_opts -p1 < $0;;
       -unpatch) patch $patch_opts -p1 -R < $0;;
        *)
                echo >&2 "`basename $0`: script expects -patch|-unpatch as 
argument"
                exit 1;;
esac

exit 0

diff -ru hylafax-4.2.1/util/faxcron.sh.in hylafax-4.2.1.new/util/faxcron.sh.in
--- hylafax-4.2.1/util/faxcron.sh.in    2004-12-22 14:44:06.000000000 +0100
+++ hylafax-4.2.1.new/util/faxcron.sh.in        2005-08-06 12:24:16.000000000 
+0200
@@ -69,12 +69,18 @@
 UPDATE="date +'%D %H:%M' >$LAST"
 
 # security
-TMPDIR=/tmp/.faxcron.sh$$
-rm -rf $TMPDIR
-mkdir $TMPDIR || exit 1
+if test -n "`type -p mktemp`" ; then
+        TMPFDIR=`mktemp -d -t xferfax.XXXXXX`  || { echo "$0: Cannot create 
temporary dir!" >&2 ; exit 1; }
+else
+       TMPFDIR=${TMPDIR-/tmp}/.faxcron.sh$$
+       rm -rf $TMPFDIR
+       mkdir $TMPFDIR || { echo "$0: Cannot create temporary dir! Aborting." ; 
exit 1; }
+fi
+
+JUNK=$TMPFDIR/faxjunk$$         # temp file used multiple times
+AWKTMP=$TMPFDIR/faxawk$$                # temp file for awk program
 
-JUNK=$TMPDIR/faxjunk$$         # temp file used multiple times
-AWKTMP=$TMPDIR/faxawk$$                # temp file for awk program
+trap "$RM \$AWKTMP \$JUNK; [ -d $TMPFDIR ] && $RM -rf $TMPFDIR" 0 1 2 15
 
 while [ x"$1" != x"" ] ; do
     case $1 in
@@ -90,7 +96,6 @@
     shift
 done
 
-trap "$RM \$AWKTMP \$JUNK; $RM -rf $TMPDIR; exit 1" 0 1 2 15
 
 
 test -z "$LASTRUN" && LASTRUN=`$CAT $LAST 2>/dev/null`
@@ -285,7 +290,7 @@
 EOF
 $AWK -f $AWKTMP -v LASTRUN="$LASTRUN" TRANSCRIPT="\
     LOGFILE=log/%s;\
-    TMP=$TMPDIR/faxlog\$\$;\
+    TMP=$TMPFDIR/faxlog\$\$;\
     if [ -f \$LOGFILE ]; then\
        $SED -n -e '/%s %s %s.*SESSION BEGIN/,/SESSION END/p' \$LOGFILE |\
        $SED -e '/start.*timer/d'\
diff -ru hylafax-4.2.1/util/recvstats.sh.in 
hylafax-4.2.1.new/util/recvstats.sh.in
--- hylafax-4.2.1/util/recvstats.sh.in  2004-06-18 06:10:29.000000000 +0200
+++ hylafax-4.2.1.new/util/recvstats.sh.in      2005-08-06 12:25:17.000000000 
+0200
@@ -153,15 +153,16 @@
 
 #
 # Generate an awk program to process the statistics file.
-#
-tmpAwk=/tmp/xferfax$$
-trap "rm -f $tmpAwk; exit 1" 0 1 2 15
-
-# security
-rm -rf $tmpAwk
-${NOCLOBBER_ON}
-> $tmpAwk || exit 1
-${NOCLOBBER_OFF}
+if test -n "`type -p mktemp`" ; then
+        tmpAwk=`mktemp -t xferfax.XXXXXX`  || { echo "$0: Cannot create 
temporary file!" >&2 ; exit 1; }
+else
+        tmpAwk=${TMPDIR-/tmp}/xferfax$$
+        rm -f $tmpAwk
+        ${NOCLOBBER_ON}
+        > $tmpAwk || { echo "$0: Cannot create temporary file! Aborting." ; 
exit 1; }
+        ${NOCLOBBER_OFF}
+fi
+trap "[ -f $tmpAwk ] && rm -f $tmpAwk" 0 1 2 15
 
 ($CAT<<'EOF'
 #
diff -ru hylafax-4.2.1/util/xferfaxstats.sh.in 
hylafax-4.2.1.new/util/xferfaxstats.sh.in
--- hylafax-4.2.1/util/xferfaxstats.sh.in       2004-06-18 06:10:29.000000000 
+0200
+++ hylafax-4.2.1.new/util/xferfaxstats.sh.in   2005-08-06 12:25:28.000000000 
+0200
@@ -26,6 +26,9 @@
 # LIABILITY, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE 
 # OF THIS SOFTWARE.
 #
+# security
[EMAIL PROTECTED]@
[EMAIL PROTECTED]@
 
 #
 # Print Statistics about Transmitted Facsimile.
@@ -169,8 +172,17 @@
 #
 # Generate an awk program to process the statistics file.
 #
-tmpAwk=/tmp/xferfax$$
-trap "rm -f $tmpAwk; exit 1" 0 1 2 15
+if test -n "`type -p mktemp`" ; then
+        tmpAwk=`mktemp -t xferfax.XXXXXX`  || { echo "$0: Cannot create 
temporary file!" >&2 ; exit 1; }
+else
+       tmpAwk=${TMPDIR-/tmp}/xferfax$$
+       rm -f $tmpAwk
+       ${NOCLOBBER_ON}
+       > $tmpAwk || { echo "$0: Cannot create temporary file! Aborting." ; 
exit 1; }
+       ${NOCLOBBER_OFF}
+fi
+trap "[ -f $tmpAwk ] && rm -f $tmpAwk" 0 1 2 15
+
 
 ($CAT<<'EOF'
 #

Attachment: signature.asc
Description: Digital signature

Reply via email to