* lib/fileutils.rb (remove_entry_secure): does not use chdir(2).

git-svn-id: http://svn.ruby-lang.org/repos/ruby/trunk@9214 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

2 files changed, 35 insertions, 24 deletions

diff --git a/ChangeLog b/ChangeLog
index a41647780..fbba2c1a9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+Mon Sep 19 05:32:41 2005  Minero Aoki  <aamine@loveruby.net>
+
+	* lib/fileutils.rb (remove_entry_secure): does not use chdir(2).
+
 Mon Sep 19 03:17:48 2005  Tanaka Akira  <akr@m17n.org>
 
         * file.c (rb_thread_flock): wrap the flock system call by
diff --git a/lib/fileutils.rb b/lib/fileutils.rb
index e7a4e0cf0..e3fbabce0 100644
--- a/lib/fileutils.rb
+++ b/lib/fileutils.rb
@@ -623,9 +623,6 @@ module FileUtils
   # user (root) should invoke this method.  Otherwise this method does not
   # work.
   #
-  # WARNING: remove_entry_secure uses chdir(2), this method is not
-  # multi-thread safe, nor reentrant.
-  #
   # For details of this security vulnerability, see Perl's case:
   #
   #   http://www.cve.mitre.org/cgi-bin/cvename.cgi?name=CAN-2005-0448
@@ -634,40 +631,41 @@ module FileUtils
   # For fileutils.rb, this vulnerability is reported in [ruby-dev:26100].
   #
   def remove_entry_secure(path, force = false)
+    unless fu_have_symlink?
+      remove_entry path, force
+      return
+    end
     fullpath = File.expand_path(path)
-    st = File.stat(File.dirname(fullpath))
-    unless st.world_writable?
+    st = File.lstat(fullpath)
+    unless st.directory?
+      File.unlink fullpath
+      return
+    end
+    # is a directory.
+    parent_st = File.stat(File.dirname(fullpath))
+    unless parent_st.world_writable?
       remove_entry path, force
       return
     end
-    unless st.sticky?
-      raise ArgumentError, "parent directory is insecure: #{path}"
+    unless parent_st.sticky?
+      raise ArgumentError, "parent directory is world writable, FileUtils#remove_entry_secure does not work; abort: #{path.inspect} (parent directory mode #{'%o' % parent_st.mode})"
     end
+    # freeze tree root
     euid = Process.euid
-    prevcwd = Dir.getwd
-    begin
-      begin
-        Dir.chdir(fullpath)
-      rescue SystemCallError
-        # seems a file
+    File.open(fullpath + '/.') {|f|
+      unless fu_stat_identical_entry?(st, f.stat)
+        # symlink (TOC-to-TOU attack?)
         File.unlink fullpath
         return
       end
-      unless fu_stat_identical_entry?(File.lstat(fullpath), File.stat('.'))
-        # seems TOC-to-TOU attack
-        File.unlink fullpath
-        return
-      end
-      File.chown euid, nil, '.'
-      File.chmod 0700, '.'
-    ensure
-      Dir.chdir prevcwd
-    end
+      f.chown euid, -1
+      f.chmod 0700
+    }
     # ---- tree root is frozen ----
     root = Entry_.new(path)
     root.preorder_traverse do |ent|
       if ent.directory?
-        ent.chown euid, nil
+        ent.chown euid, -1
         ent.chmod 0700
       end
     end
@@ -682,9 +680,18 @@ module FileUtils
     raise unless force
   end
 
+  def fu_have_symlink?
+    File.symlink nil, nil
+  rescue NotImplementedError
+    return false
+  rescue
+    return true
+  end
+
   def fu_stat_identical_entry?(a, b)
     a.dev == b.dev and a.ino == b.ino
   end
+  private :fu_stat_identical_entry?
 
   #
   # This method removes a file system entry +path+.