After my previous post, I realised that there are lots of other places in the code where mktemp is used and more importantly, alternate unsafe code is used. So I have had a crack at cleaning things up.
------------------------------------------------------------------------ There are currently at least two problems with maketempfile. Firstly, there is a race in the following constrct: rm -f "$F"; touch "$F" As an attacker could potitinally create a symlink to "$F" between the call to rm and the call to touch. Secondly the use of $RANDOM appears to be a bashism. On dash its usage in BasicSanityCheck appears to evaluate to the empty string. See Debian Bug #489607, http://bugs.debian.org/489607 This patch takes the approach of using mkdir, which is atomic, to create a safe place to store the logfile the tempoary directory that is used by the filesystem check. The patch also makes sure that the return value is checked and the script exits cleanly if an error occurs. This code is always used, there seems no point is providing a robust fallback to mktemp - if its robust, it can be used :-) For a discussion of creating tempoary files in shell see http://www.linuxsecurity.com/content/view/115462/81/ Signed-off-by: Simon Horman <[EMAIL PROTECTED]> --- Please note that this patch adds the inclusion of ha.d/shellfuncs to tsa_plugin/linuxha-adapter.in. It would also benifit from doing the same to BasicSanityCheck. Index: heartbeat/heartbeat/lib/BasicSanityCheck.in =================================================================== --- heartbeat.orig/heartbeat/lib/BasicSanityCheck.in 2008-08-06 18:08:05.000000000 +1000 +++ heartbeat/heartbeat/lib/BasicSanityCheck.in 2008-08-06 18:50:19.000000000 +1000 @@ -49,7 +49,6 @@ CRMTEST="@PYTHON@ $SCRIPTDIR/cts/CTSlab. SNMPAGENTTEST=$SCRIPTDIR/SNMPAgentSanityCheck BASE64_MD5_TEST=$HBLIB/base64_md5_test MALLOC_CHECK_=2; export MALLOC_CHECK_ [EMAIL PROTECTED]@ [EMAIL PROTECTED]@ # IDENTSTRING="Linux-HA TEST configuration file - REMOVEME!!" @@ -64,35 +63,28 @@ SIGLIST="0 1 2 3 6 15" errcount=0 -# Make temp files the paranoid way... -maketempfile() { -# -# Use mktemp if we have it, otherwise... -# -# Construct a difficult-to-guess filename if we don't -# Make sure non-mktemp files can't be subverted -# $RANDOM is not strictly necessary, but nice to have... -# - if - test "x$MKTEMP" != "x" \ - && F=`$MKTEMP /tmp/lha-XXXXXX` && [ ! -z "$F" -a -f "$F" ] - then - echo $F - else - while - echo >/dev/null & - F=/tmp/lha-${RANDOM}-$$-$! - rm -f "$F"; touch "$F" - # Try again if we don't own it, or it's a symlink - # Or somehow not a regular file... - $TESTPROG ! -O "$F" -o -L "$F" -o ! -f "$F" - do - : Try again... - done - echo $F - fi +# Make a safe place to store logs +maketempdir() +{ + local i=0 + local tmp + + while [ $i -gt 0 ]; do + tmp="/tmp/lha-dir-$$-$i" + if (umask 077 && mkdir "$tmp"); then + echo "$tmp" + return 0 + fi + i=$((i+1)) + done + + echo "Could not create tempoary directory to store logs" >& 2 + return 1 } -LOGFILE=`maketempfile` + +LOGDIR=`maketempdir` || exit 1 +LOGFILE="$LOGDIR/log" +touch "$LOGFILE" cd $HADIR ulimit -c unlimited @@ -328,6 +320,7 @@ SaveLog() { SAVELOG=/tmp/linux-ha.testlog chmod a+r $LOGFILE mv $LOGFILE $SAVELOG + rm -rf "$LOGDIR" echo "$errcount errors. Log file is stored in $SAVELOG" } @@ -985,7 +978,8 @@ TestRA() { fi if [ `uname -s` = 'Linux' ]; then - [EMAIL PROTECTED]@ -d /tmp/lha-dir-XXXXXXXXXXX` + MNT_DIR="$LOGDIR/mnt" + mkdir "$MNT_DIR" echo "Testing RA: Filesystem" | tee -a $LOGFILE $OCF_TESTER -o device=/dev/null -o fstype=proc -o directory=$MNT_DIR \ -n DemoFS $RADIR/Filesystem >>$LOGFILE 2>&1 Index: heartbeat/heartbeat/shellfuncs.in =================================================================== --- heartbeat.orig/heartbeat/shellfuncs.in 2008-08-06 18:08:05.000000000 +1000 +++ heartbeat/heartbeat/shellfuncs.in 2008-08-06 18:08:18.000000000 +1000 @@ -30,7 +30,6 @@ export HA_DIR HA_RCDIR HA_FIFO HA_BIN export HA_DEBUGLOG HA_LOGFILE HA_LOGFACILITY export HA_DATEFMT HA_RESOURCEDIR HA_DOCDIR [EMAIL PROTECTED]@ [EMAIL PROTECTED]@ PATH=$HA_BIN:${HA_SBIN_DIR}:@HA_NOARCHDATAHBDIR@:$PATH PATH=`echo $PATH | sed -e 's%::%%' -e 's%:\.:%:%' -e 's%^:%%' -e 's%^\.:%%'` @@ -226,33 +225,98 @@ ha_pseudo_resource() esac } -# Make temp files the paranoid way... -maketempfile() { -# -# Use mktemp if we have it, otherwise... -# -# Construct a difficult-to-guess filename if we don't -# Make sure non-mktemp files can't be subverted -# $RANDOM is not strictly necessary, but nice to have... -# - if - test "x$MKTEMP" != "x" \ - && F=`$MKTEMP /tmp/lha-XXXXXX` && [ ! -z "$F" -a -f "$F" ] - then - echo $F - else - while - echo >/dev/null & - F=/tmp/lha-${RANDOM}-$$-$! - rm -f "$F"; touch "$F" - # Try again if we don't own it, or it's a symlink - # Or somehow not a regular file... - $TESTPROG ! -O "$F" -o -L "$F" -o ! -f "$F" - do - : Try again... - done - echo $F - fi +# usage: dirname DIR +dirname() +{ + local a + local b + + [ $# = 1 ] || return 1 + a="$1" + while [ 1 ]; do + b="${a%/}" + [ "$a" = "$b" ] && break + a="$b" + done + b=${a%/*} + [ -z "$b" -o "$a" = "$b" ] && b="." + + echo "$b" + return 0 +} + +# usage: maketempdir +maketempdir() +{ + local i=1 + local tmp + + # The loop allows for the cases where + # A process has multiple tempfiles or dirs or; + # A previous process with the same PID that had tempfiles or + # directories did not exit cleanly + while [ $i -gt 0 ]; do + tmp="/tmp/lha-dir-$$-$i" + if (umask 077 && mkdir "$tmp" 2>/dev/null); then + echo "$tmp" + return 0 + fi + i=$((i+1)) + done + + echo "Could not create tempoary directory to store logs" >& 2 + return 1 +} + +# usage: rmtempdir TMPDIR +rmtempdir() +{ + [ $# = 1 ] || return 1 + if [ -e "$1" ]; then + rmdir "$1" || return 1 + fi + return 0 +} + +# usage: maketempfile [-d] +maketempfile() +{ + local dir="no" + local name + + if [ $# = 1 -a "$1" = "-d" ]; then + dir="yes" + elif [ $# != 0 ]; then + return 1 + fi + + name=$(maketempdir) || return $? + + if [ "$dir" = "yes" ]; then + echo "$name" + return 0 + fi + + name="$name/tmpfile" + + if ! touch "$name"; then + rmtempdir + return 1 + fi + + echo "$name" + return 0 +} + +# usage: rmtempfile TMPFILE +rmtempfile () +{ + [ $# = 1 ] || return 1 + if [ -e "$1" ]; then + rm "$1" || return 1 + fi + rmtempdir $(dirname "$1") || return 1 + return 0 } # The 'ping' command takes highly OS-dependent arguments, so this Index: heartbeat/lib/plugins/stonith/external/hmchttp.in =================================================================== --- heartbeat.orig/lib/plugins/stonith/external/hmchttp.in 2008-08-06 18:08:05.000000000 +1000 +++ heartbeat/lib/plugins/stonith/external/hmchttp.in 2008-08-06 18:42:42.000000000 +1000 @@ -28,9 +28,8 @@ #set -x hostlist=`echo $hostlist | tr ',' ' '` -COOKIEFILE=`maketempfile` -#trap "cat $COOKIEFILE >&2; rm -f $COOKIEFILE" EXIT -trap "rm -f $COOKIEFILE" EXIT +trap '[ ! -e "$COOKIEFILE" ] || rmtempfile "$COOKIEFILE"' 0 +COOKIEFILE=`maketempfile` || exit 1 : ${CURLBIN="/usr/bin/curl"} : ${user=admin} Index: heartbeat/lib/plugins/stonith/external/ibmrsa.in =================================================================== --- heartbeat.orig/lib/plugins/stonith/external/ibmrsa.in 2008-08-06 18:08:05.000000000 +1000 +++ heartbeat/lib/plugins/stonith/external/ibmrsa.in 2008-08-06 18:44:27.000000000 +1000 @@ -28,8 +28,8 @@ #echo "started with: $@" | debug . @HA_HBCONF_DIR@/shellfuncs -outf=`maketempfile` -trap "cat $outf >&2; rm -f $outf" EXIT +trap 'if [ -n "$outf" ]; then cat "$outf" >&2; rmtempfile "$outf"; fi' 0 +outf=`maketempfile` || exit 1 chkmpcli() { test -x /opt/IBMmpcli/bin/MPCLI.sh Index: heartbeat/resources/OCF/WAS =================================================================== --- heartbeat.orig/resources/OCF/WAS 2008-08-06 18:08:05.000000000 +1000 +++ heartbeat/resources/OCF/WAS 2008-08-06 18:45:20.000000000 +1000 @@ -325,7 +325,8 @@ WAS_report_status() { # This is actually faster than WAS_status above... # WAS_monitor() { - tmpfile=`maketempfile` + trap '[ -z "$tmpfile" ] || rmtempfile "$tmpfile"' 0 + tmpfile=`maketempfile` || return 1 SnoopPort=`GetWASSnoopPort $1` output=`$WGET -nv -O$tmpfile http://localhost:$SnoopPort/servlet/snoop 2>&1` rc=$? @@ -344,7 +345,6 @@ WAS_monitor() { ocf_log "err" "WAS: $1: wget failure: $output" rc=$OCF_ERR_GENERIC fi - rm -fr $tmpfile return $rc } Index: heartbeat/resources/OCF/WAS6 =================================================================== --- heartbeat.orig/resources/OCF/WAS6 2008-08-06 18:08:05.000000000 +1000 +++ heartbeat/resources/OCF/WAS6 2008-08-06 18:46:54.000000000 +1000 @@ -294,7 +294,8 @@ WAS_report_status() { # This is actually faster than WAS_status above... # WAS_monitor() { - tmpfile=`maketempfile` + trap '[ -z "$tmpfile" || rmtempfile "$tmpfile"' 0 + tmpfile=`maketempfile` || exit 1 SnoopPort=`GetWASSnoopPort $1` output=`$WGET -nv -O$tmpfile http://localhost:$SnoopPort/snoop 2>&1` rc=$? @@ -313,7 +314,6 @@ WAS_monitor() { ocf_log "err" "WAS: $1: wget failure: $output" rc=$OCF_ERR_GENERIC fi - rm -fr $tmpfile return $rc } Index: heartbeat/tools/hb_report.in =================================================================== --- heartbeat.orig/tools/hb_report.in 2008-08-06 18:08:05.000000000 +1000 +++ heartbeat/tools/hb_report.in 2008-08-06 18:47:53.000000000 +1000 @@ -399,6 +399,7 @@ checklogs() { logs=$(find $1 -name $HALOG_F; for l in $EXTRA_LOGS; do find $1/* -name `basename $l`; done) [ "$logs" ] || return + trap '[ -z "$patfiles" ] || rmtempfile "$patfiles"]' 0 pattfile=`maketempfile` || fatal "cannot create temporary files" for p in $LOG_PATTERNS; do @@ -409,7 +410,8 @@ checklogs() { for n in `getnodes`; do cat $logs | grep -f $pattfile done - rm -f $pattfile + rmtempfile $patfiles + trap "" 0 } # Index: heartbeat/tools/utillib.sh =================================================================== --- heartbeat.orig/tools/utillib.sh 2008-08-06 18:08:06.000000000 +1000 +++ heartbeat/tools/utillib.sh 2008-08-06 18:48:40.000000000 +1000 @@ -203,6 +203,12 @@ touchfile() { perl -e "\$file=\"$t\"; \$tm=$1;" -e 'utime $tm, $tm, $file;' && echo $t } +find_files_clean() { + [ -z "$to_stamp" ] || rmtempfile "$to_stamp" + to_stamp="" + [ -z "$from_stamp" ] || rmtempfile "$from_stamp" + from_stamp="" +} find_files() { dir=$1 from_time=$2 @@ -211,14 +217,24 @@ find_files() { warning "sorry, can't find files based on time if you don't supply time" return } - from_stamp=`touchfile $from_time` + trap find_files_clean 0 + if ! from_stamp=`touchfile $from_time`; then + warning "sorry, can't create temoary file for find_files" + return + fi findexp="-newer $from_stamp" if isnumber "$to_time" && [ "$to_time" -gt 0 ]; then - to_stamp=`touchfile $to_time` + if ! to_stamp=`touchfile $to_time`; then + warning "sorry, can't create temoary file for" \ + "find_files" + find_files_clean + return + fi findexp="$findexp ! -newer $to_stamp" fi find $dir -type f $findexp - rm -f $from_stamp $to_stamp + find_files_clean + trap "" 0 } # @@ -311,6 +327,12 @@ sanitize_hacf() { {print} ' } +sanitize_one_clean() { + [ -z "$tmp" ] || rmtempfile "$tmp" + tmp="" + [ -z "$ref" ] || rmtempfile "$ref" + ref="" +} sanitize_one() { file=$1 compress="" @@ -322,8 +344,13 @@ sanitize_one() { compress=cat decompress=cat fi - tmp=`maketempfile` && ref=`maketempfile` || + trap sanitize_one_clean 0 + tmp=`maketempfile` + ref=`maketempfile` + if [ -z "$tmp" -o -z "$ref" ]; then + sanitize_one_clean fatal "cannot create temporary files" + fi touch -r $file $ref # save the mtime if [ "`basename $file`" = ha.cf ]; then sanitize_hacf @@ -331,8 +358,10 @@ sanitize_one() { $decompress | sanitize_xml_attrs | $compress fi < $file > $tmp mv $tmp $file + tmp="" touch -r $ref $file - rm -f $ref + sanitize_one_clean + trap "" 0 } # Index: heartbeat/resources/OCF/.ocf-binaries.in =================================================================== --- heartbeat.orig/resources/OCF/.ocf-binaries.in 2008-08-06 18:08:17.000000000 +1000 +++ heartbeat/resources/OCF/.ocf-binaries.in 2008-08-06 18:08:32.000000000 +1000 @@ -11,7 +11,6 @@ export PATH : ${EGREP:="@EGREP@"} : ${IFCONFIG_A_OPT:="@IFCONFIG_A_OPT@"} : ${MAILCMD:[EMAIL PROTECTED]@} -: ${MKTEMP:[EMAIL PROTECTED]@} : ${PING:[EMAIL PROTECTED]@} : ${RPM:[EMAIL PROTECTED]@} : ${SH:[EMAIL PROTECTED]@} @@ -48,7 +47,6 @@ export PATH : ${SCP:=scp} : ${SSH:=ssh} : ${SWIG:=swig} -: ${MKTEMP:=mktemp} : ${GZIP_PROG:=gzip} : ${TAR:=tar} : ${MD5:=md5} Index: heartbeat/tsa_plugin/linuxha-adapter.in =================================================================== --- heartbeat.orig/tsa_plugin/linuxha-adapter.in 2008-08-06 18:08:44.000000000 +1000 +++ heartbeat/tsa_plugin/linuxha-adapter.in 2008-08-06 18:49:12.000000000 +1000 @@ -17,6 +17,8 @@ MSG_NOJAVA='Cannot find Java - exit.' UNAME=`uname -s` +. @sysconfdir@/ha.d/shellfuncs + function logit () { if [ $LOG_LVL -ge $1 ] @@ -114,22 +116,14 @@ function startAdapter () RC=$? if [ $RC -eq $OFFLINE ] ; then if [ "$UNAME" = AIX ] ; then - MKTEMP='' LSOF='' else #linux - MKTEMP=`whereis mktemp | cut -d' ' -f2 | cut -d' ' -f3` LSOF=`whereis lsof | cut -d' ' -f2 | cut -d' ' -f3` fi rm -f /tmp/$adapter.* # Create a tempfile to capture the first few lines of the output - if [ -n "$MKTEMP" ] ; then - TMPFILE=`$MKTEMP -q /tmp/$adapter.XXXXXX` - else - # It's better to use mktemp, but we don't have it, we make it cryptic - RNDM=$(echo $RANDOM) - TMPFILE=/tmp/$adapter.XXXXXX-RNDM - touch $TMPFILE - fi + trap '[ -z "$TMPFILE" ] || rmtempfile "$TMPFILE"' 0 + TMPFILE=`maketempfile` || exit 1 # set LIBRAY path for jni parts NATIVE_LIBPATH=/lib:/usr/lib if [ "$UNAME" = "AIX" ] ; then @@ -163,7 +157,7 @@ function startAdapter () fi # cat the output to the screen and remove the tempfile - cat $TMPFILE && rm -f $TMPFILE + cat $TMPFILE echo "$adapter has started." logit 0 "$adapter has started." RC=0 Index: heartbeat/configure.in =================================================================== --- heartbeat.orig/configure.in 2008-08-06 18:18:30.000000000 +1000 +++ heartbeat/configure.in 2008-08-06 18:19:47.000000000 +1000 @@ -588,20 +588,14 @@ AC_PATH_PROGS(PKGCONFIG, pkg-config) dnl ************************************************************************ dnl Check whether non-root user can chown. dnl ************************************************************************ -AC_PATH_PROGS(MKTEMP, mktemp) if test -n "$WHOAMI"; then IAM=`$WHOAMI` fi AC_MSG_CHECKING(if chown works for non-root) -dnl Prefer "mktemp" command. But some OSes lack it; they can "touch". -if test -n "$MKTEMP"; then - F=`$MKTEMP "./.chown-testXXXXX"` -else - F="./.chown-test.$$" - touch $F -fi +F="./.chown-test.$$" +touch $F if case "$IAM" in _______________________________________________________ Linux-HA-Dev: [email protected] http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev Home Page: http://linux-ha.org/
