[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. puppet-0.24.5-rc3-1601-gf8c1b08

[James Turnbull]

The following commit has been merged in the upstream branch:
commit 6111ba80f2c6f6d1541af971f565119e6e03d77d
Author: Markus Roberts <Markus at reality.com>
Date:   Sun Jan 3 19:04:29 2010 -0800

    Fix for temporary file security whole
    
    We create temporary files in /tmp/ with predictable names.  These
    could be used by an attacker to DoS a box by setting a symlink to
    some other file (say, /etc/shadow) and waiting for us to overwrite
    it.
    
    The minimalistic solution employed by this patch is to wrap all such
    file writing with a paranoid wrapper that:
    
    1) Check to see if the target exists
    2) Issues a warning if it was a symlink
    3) Deletes it
    4) Waits (0.1 seconds if it was a file, 5 seconds if it was a symlink)
    5) Opens the file with EXCL, which will fail if the file has come back.
    
    If this succeeds (as it normally will) it has exactly the same semantics
    as the original code (a must, as we are right at a release boundary).
    However, under no circumstances will it follow a preexisting symlink (the
    operating system guarantees this with EXCL) so the danger of an exploit
    has been converted into the possibility of a failure, with an appropriate
    warning.

diff --git a/lib/puppet/daemon.rb b/lib/puppet/daemon.rb
index 16b1f80..0f538fe 100755
--- a/lib/puppet/daemon.rb
+++ b/lib/puppet/daemon.rb
@@ -31,10 +31,10 @@ class Puppet::Daemon
             $stderr.reopen $stdout
             Puppet::Util::Log.reopen
         rescue => detail
-            File.open("/tmp/daemonout", "w") { |f|
+            Puppet.err "Could not start %s: %s" % [Puppet[:name], detail]
+            Puppet::Util::secure_open("/tmp/daemonout", "w") { |f|
                 f.puts "Could not start %s: %s" % [Puppet[:name], detail]
             }
-            Puppet.err "Could not start %s: %s" % [Puppet[:name], detail]
             exit(12)
         end
     end
diff --git a/lib/puppet/network/server.rb b/lib/puppet/network/server.rb
index f21254b..01a55df 100644
--- a/lib/puppet/network/server.rb
+++ b/lib/puppet/network/server.rb
@@ -22,7 +22,7 @@ class Puppet::Network::Server
             $stderr.reopen $stdout
             Puppet::Util::Log.reopen
         rescue => detail
-            File.open("/tmp/daemonout", "w") { |f|
+            Puppet::Util.secure_open("/tmp/daemonout", "w") { |f|
                 f.puts "Could not start %s: %s" % [Puppet[:name], detail]
             }
             raise "Could not start %s: %s" % [Puppet[:name], detail]
diff --git a/lib/puppet/rails/benchmark.rb b/lib/puppet/rails/benchmark.rb
index c33b2fb..1fbd011 100644
--- a/lib/puppet/rails/benchmark.rb
+++ b/lib/puppet/rails/benchmark.rb
@@ -64,6 +64,6 @@ module Puppet::Rails::Benchmark
             data = {}
         end
         data[branch] = $benchmarks
-        File.open(file, "w") { |f| f.print YAML.dump(data) }
+        Puppet::Util.secure_open(file, "w") { |f| f.print YAML.dump(data) }
     end
 end
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 3752a6d..e2df5c3 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -407,6 +407,28 @@ module Util
     end
 
     module_function :memory, :thinmark
+
+    def secure_open(file,must_be_w,&block)
+        raise Puppet::DevError,"secure_open only works with mode 'w'" unless must_be_w == 'w'
+        raise Puppet::DevError,"secure_open only requires a block"    unless block_given?
+        Puppet.warning "#{file} was a symlink to #{File.readlink(file)}" if File.symlink?(file)
+        if File.exists?(file) or File.symlink?(file)
+            wait = File.symlink?(file) ? 5.0 : 0.1
+            File.delete(file)
+            sleep wait # give it a chance to reappear, just in case someone is actively trying something.
+        end
+        begin
+            File.open(file,File::CREAT|File::EXCL|File::TRUNC|File::WRONLY,&block)
+        rescue Errno::EEXIST
+            desc = File.symlink?(file) ? "symlink to #{File.readlink(file)}" : File.stat(file).ftype
+            puts "Warning: #{file} was apparently created by another process (as"
+            puts "a #{desc}) as soon as it was deleted by this process.  Someone may be trying"
+            puts "to do something objectionable (such as tricking you into overwriting system"
+            puts "files if you are running as root)."
+            raise
+        end
+    end
+    module_function :secure_open
 end
 end
 
diff --git a/lib/puppet/util/reference.rb b/lib/puppet/util/reference.rb
index 7526e65..93542df 100644
--- a/lib/puppet/util/reference.rb
+++ b/lib/puppet/util/reference.rb
@@ -36,7 +36,7 @@ class Puppet::Util::Reference
 
     def self.pdf(text)
         puts "creating pdf"
-        File.open("/tmp/puppetdoc.txt", "w") do |f|
+        Puppet::Util.secure_open("/tmp/puppetdoc.txt", "w") do |f|
             f.puts text
         end
         rst2latex = %x{which rst2latex}
@@ -48,6 +48,9 @@ class Puppet::Util::Reference
         end
         rst2latex.chomp!
         cmd = %{#{rst2latex} /tmp/puppetdoc.txt > /tmp/puppetdoc.tex}
+        Puppet::Util.secure_open("/tmp/puppetdoc.tex","w") do |f|
+            # If we get here without an error, /tmp/puppetdoc.tex isn't a tricky cracker's symlink
+        end
         output = %x{#{cmd}}
         unless $? == 0
             $stderr.puts "rst2latex failed"
@@ -67,7 +70,7 @@ class Puppet::Util::Reference
         puts "Creating markdown for #{name} reference."
         dir = "/tmp/" + Puppet::PUPPETVERSION
         FileUtils.mkdir(dir) unless File.directory?(dir) 
-        File.open(dir + "/" + "#{name}.rst", "w") do |f|
+        Puppet::Util.secure_open(dir + "/" + "#{name}.rst", "w") do |f|
             f.puts text
         end
         pandoc = %x{which pandoc}
@@ -190,7 +193,7 @@ class Puppet::Util::Reference
     end
 
     def trac
-        File.open("/tmp/puppetdoc.txt", "w") do |f|
+        Puppet::Util.secure_open("/tmp/puppetdoc.txt", "w") do |f|
             f.puts self.to_trac
         end
 

-- 
Puppet packaging for Debian