Skip to content

Commit

Permalink
(#9792) Predictable temporary filename in ralsh.
Browse files Browse the repository at this point in the history
When ralsh is used in edit mode the temporary filename is in a shared
directory, and is absolutely predictable.  Worse, it won't be touched until
well after the startup of the command.

It can be tricked into writing through a symlink to edit any file on the
system, or to create through it, but worse - the file is reopened with the
same name later, so it can have the target replaced between edit and
operate...

The only possible mitigation comes from the system editor and the behaviour it
has around editing through symbolic links, which is very weak.

This improves this to prefer the current working directory for the temporary
file, and to be somewhat less predictable and more safe in conjuring it into
being.

Fixes CVE-2011-3871

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
  • Loading branch information
Daniel Pittman authored and stahnma committed Sep 29, 2011
1 parent b29b178 commit d76c309
Showing 1 changed file with 17 additions and 10 deletions.
27 changes: 17 additions & 10 deletions lib/puppet/application/resource.rb
Expand Up @@ -190,18 +190,25 @@ def main
end.map(&format).join("\n")

if options[:edit]
file = "/tmp/x2puppet-#{Process.pid}.pp"
require 'tempfile'
# Prefer the current directory, which is more likely to be secure
# and, in the case of interactive use, accessible to the user.
tmpfile = Tempfile.new('x2puppet', Dir.pwd)
begin
File.open(file, "w") do |f|
f.puts text
end
ENV["EDITOR"] ||= "vi"
system(ENV["EDITOR"], file)
system("puppet -v #{file}")
# sync write, so nothing buffers before we invoke the editor.
tmpfile.sync = true
tmpfile.puts text

# edit the content
system(ENV["EDITOR"] || 'vi', tmpfile.path)

# ...and, now, pass that file to puppet to apply. Because
# many editors rename or replace the original file we need to
# feed the pathname, not the file content itself, to puppet.
system('puppet -v ' + tmpfile.path)
ensure
#if FileTest.exists? file
# File.unlink(file)
#end
# The temporary file will be safely removed.
tmpfile.close(true)
end
else
puts text
Expand Down

0 comments on commit d76c309

Please sign in to comment.