[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#321473: metamail: DoS to users to prevent usage of showpartial through _hard_ links

Package: metamail
Version: 2.7-47
Priority: normal
Tags: security patch

While reviewing scripts for instances of symlink vulnerabilities I've found
a possible denial of service condition for users that try to use showpartial.
The showpartial script of metamail has a temporary hardcoded path
for the users downloaded messages set to $METAMAIL_TMPDIR/msg-parts-`whoami`
with $METAMAIL_TMPDIR being /tmp unless otherwise defined.

In order to prevent symlink attacks the script does this:

mkdir $TREEROOT 2>/dev/null || true
if OUTPUT=$(find $TREEROOT -maxdepth 0 -user `whoami` -print 2>/dev/null) &&
        [ -n $OUTPUT ]
        echo mkdir $TREEROOT failed
        exit 1

Which basicly creates the temporary directory and tries to determine
if it indeed belongs to the running user. If it is not able to create
it, or it doesn't belong to the running user it bails out.

The script, however, does not cope with the fact that a rogue user could
just create _hard_ (not symbolic) links to files in the same partition
belonging to the user which tries to run this script to the temporary
file. If he does this, then a msg-parts-$USER file would exist that belongs
to the user (so find returns it). In this situation, the script
is never able to go ahead and do its work and, thus, a rogue user
can prevent execution of this script for all $USERs in a system.

It would be best if the script tested for this case. The attached patch
copes with this by removing TREEROOT if it exists but is not a directory.

I believe using mktemp in the script is not possible since the directory
needs to be present in succesive runs. An alternative would be to 
use mktemp to set the temporary dir and store this information in the
user's directory (~/.metamail_treeroot for example). Just for the fun
of it, attached is also a patch that implements that alternative.

Even better, the script could also try to use safer temporary directories
(for example, if $TMPDIR has been defined by the user and exists)
instead of introducing yet another environment variable for temporary
directories. But it seems that all the source code of this (ancient)
package uses the METAMAIL_TMPDIR variable.



--- showpartial.orig	2005-08-05 20:07:50.000000000 +0200
+++ showpartial	2005-08-05 20:10:21.000000000 +0200
@@ -52,6 +52,7 @@
+[ -e $TREEROOT ] && [ ! -d $TREEROOT ] && rm -f $TREEROOT
 mkdir $TREEROOT 2>/dev/null || true
 if OUTPUT=$(find $TREEROOT -maxdepth 0 -user `whoami` -print 2>/dev/null) &&
 	[ -n $OUTPUT ]
--- showpartial.orig	2005-08-05 20:07:50.000000000 +0200
+++ showpartial	2005-08-05 20:28:52.000000000 +0200
@@ -30,7 +30,7 @@
 if test -z "$3" -o ! -z "$5"
@@ -52,16 +52,19 @@
-mkdir $TREEROOT 2>/dev/null || true
-if OUTPUT=$(find $TREEROOT -maxdepth 0 -user `whoami` -print 2>/dev/null) &&
-	[ -n $OUTPUT ]
-	:
+if [ -n "$HOME" ] ; then
+# If we have a home directory, create a temporary dir and
+# store the name there unless the file already exists
+# and the directory it points to is valid directory
+	[ -e "$HOME/.metamail_treeroot" ] && TREEROOT=`cat $HOME/.metamail_treeroot`
+	[ -e "$TREEROOT" ] && [ ! -d "$TREEROOT" ] && rm -f "$TREEROOT" 
+	if [ ! -e "$HOME/.metamail_treeroot" ] || [ ! -e "$TREEROOT" ]; then
+		TREEROOT=`mktemp -d -t msg-parts.XXXXXX` || { echo "$0: Cannot create temporary dir!" >&2 ; exit 1; }
+		echo $TREEROOT >$HOME/.metamail_treeroot
+	fi
-	echo mkdir $TREEROOT failed
-	exit 1
+	TREEROOT=$METAMAIL_TMPDIR/msg-parts-`whoami`
 if test ! -d "${TREEROOT}/$id"
 	mkdir "${TREEROOT}/$id"

Attachment: signature.asc
Description: Digital signature

Reply to: