Skip to content

Commit

Permalink
Fix travis failures in fileutils due to smarter tmpdir.
Browse files Browse the repository at this point in the history
Travis started failing after we updated tmpdir to allow using a
world-writable dir if sticky bit is set. The bug that was exposed
lies in FileUtils.remove_entry_secure, where it attempts to open
the "." entry in the target directory primarily to stat, chmod,
and chown it. Because JVM cannot open a stream to a directory
entry, JRuby can't do it either, and because we started allowing
tmpdir to use sticky-bit world-writable dirs, this logic started
getting hit for Dir.mktmpdir's cleanup.

The fix was to directly stat, chown, and chmod the "." entry
rather than opening it as a file, since these operations end up
equivalent.

Note of clarification: this issue has always existed, but most
uses of remove_entry_secure were either already in secure dirs
(non-world-writable) or insecure dirs with no sticky bit (in
which case remove_entry_secure raises an error, correctly). It
only became apparent because of the recent tmpdir change.
  • Loading branch information
headius committed Apr 30, 2013
1 parent 3ad42f9 commit 610be9f
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 8 deletions.
9 changes: 5 additions & 4 deletions lib/ruby/1.8/fileutils.rb
Expand Up @@ -696,14 +696,15 @@ def remove_entry_secure(path, force = false)
end
# freeze tree root
euid = Process.euid
File.open(fullpath + '/.') {|f|
unless fu_stat_identical_entry?(st, f.stat)
dot_file = fullpath + "/."
File.lstat(dot_file).tap {|fstat|
unless fu_stat_identical_entry?(st, fstat)
# symlink (TOC-to-TOU attack?)
File.unlink fullpath
return
end
f.chown euid, -1
f.chmod 0700
File.chown euid, -1, dot_file
File.chmod 0700, dot_file
unless fu_stat_identical_entry?(st, File.lstat(fullpath))
# TOC-to-TOU attack?
File.unlink fullpath
Expand Down
9 changes: 5 additions & 4 deletions lib/ruby/1.9/fileutils.rb
Expand Up @@ -718,14 +718,15 @@ def remove_entry_secure(path, force = false)
end
# freeze tree root
euid = Process.euid
File.open(fullpath + '/.') {|f|
unless fu_stat_identical_entry?(st, f.stat)
dot_file = fullpath + "/."
File.lstat(dot_file).tap {|fstat|
unless fu_stat_identical_entry?(st, fstat)
# symlink (TOC-to-TOU attack?)
File.unlink fullpath
return
end
f.chown euid, -1
f.chmod 0700
File.chown euid, -1, dot_file
File.chmod 0700, dot_file
unless fu_stat_identical_entry?(st, File.lstat(fullpath))
# TOC-to-TOU attack?
File.unlink fullpath
Expand Down

0 comments on commit 610be9f

Please sign in to comment.