From: Rafael Garcia-Suarez <rgarciasuarez@gmail.com>
Date: Wed, 9 Feb 2005 09:28:19 +0000 (+0000)
Subject: Patch for CAN-2004-0452 by Jeroen van Wolffelaar.
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=e2ba98a1f4e37e016e9f91967cb62bf01e236655;p=p5sagit%2Fp5-mst-13.2.git

Patch for CAN-2004-0452 by Jeroen van Wolffelaar.
The rmtree() function in the perl File::Path module would remove
directories in an insecure manner which could lead to the removal
of arbitrary files and directories via a symlink attack.

p4raw-id: //depot/perl@23953
---

diff --git a/lib/File/Path.pm b/lib/File/Path.pm
index 7881b6b..450b0c7 100644
--- a/lib/File/Path.pm
+++ b/lib/File/Path.pm
@@ -33,7 +33,7 @@ to print the name of each directory as it is created
 =item *
 
 the numeric mode to use when creating the directories
-(defaults to 0777)
+(defaults to 0777), to be modified by the current umask.
 
 =back
 
@@ -84,14 +84,20 @@ than VMS is settled.  (defaults to FALSE)
 It returns the number of files successfully deleted.  Symlinks are
 simply deleted and not followed.
 
-B<NOTE:> If the third parameter is not TRUE, C<rmtree> is B<unsecure>
-in the face of failure or interruption.  Files and directories which
-were not deleted may be left with permissions reset to allow world
-read and write access.  Note also that the occurrence of errors in
-rmtree can be determined I<only> by trapping diagnostic messages
-using C<$SIG{__WARN__}>; it is not apparent from the return value.
-Therefore, you must be extremely careful about using C<rmtree($foo,$bar,0)>
-in situations where security is an issue.
+B<NOTE:> There are race conditions internal to the implementation of
+C<rmtree> making it unsafe to use on directory trees which may be
+altered or moved while C<rmtree> is running, and in particular on any
+directory trees with any path components or subdirectories potentially
+writable by untrusted users.
+
+Additionally, if the third parameter is not TRUE and C<rmtree> is
+interrupted, it may leave files and directories with permissions altered
+to allow deletion (and older versions of this module would even set
+files and directories to world-read/writable!)
+
+Note also that the occurrence of errors in C<rmtree> can be determined I<only>
+by trapping diagnostic messages using C<$SIG{__WARN__}>; it is not apparent
+from the return value.
 
 =head1 DIAGNOSTICS
 
@@ -192,11 +198,11 @@ sub rmtree {
 	(undef, undef, my $rp) = lstat $root or next;
 	$rp &= 07777;	# don't forget setuid, setgid, sticky bits
 	if ( -d _ ) {
-	    # notabene: 0777 is for making readable in the first place,
+	    # notabene: 0700 is for making readable in the first place,
 	    # it's also intended to change it to writable in case we have
 	    # to recurse in which case we are better than rm -rf for 
 	    # subtrees with strange permissions
-	    chmod(0777, ($Is_VMS ? VMS::Filespec::fileify($root) : $root))
+	    chmod($rp | 0700, ($Is_VMS ? VMS::Filespec::fileify($root) : $root))
 	      or carp "Can't make directory $root read+writeable: $!"
 		unless $safe;
 
@@ -230,7 +236,7 @@ sub rmtree {
 		print "skipped $root\n" if $verbose;
 		next;
 	    }
-	    chmod 0777, $root
+	    chmod $rp | 0700, $root
 	      or carp "Can't make directory $root writeable: $!"
 		if $force_writeable;
 	    print "rmdir $root\n" if $verbose;
@@ -252,7 +258,7 @@ sub rmtree {
 		print "skipped $root\n" if $verbose;
 		next;
 	    }
-	    chmod 0666, $root
+	    chmod $rp | 0600, $root
 	      or carp "Can't make file $root writeable: $!"
 		if $force_writeable;
 	    print "unlink $root\n" if $verbose;