(#13260) Use mktmpdir when downloading packages

This fixes a security vulnerability in the appdmg and pkgdmg providers where they would curl packages directly into /tmp and the install them, allowing an attacker to craft a symlink and overwrite arbitrary files or install arbitrary packages. Conflicts: lib/puppet/provider/package/appdmg.rb lib/puppet/provider/package/pkgdmg.rb
puppetlabs · Apr 3, 2012 · c51447d · c51447d
1 parent 568ded5
commit c51447d
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 24 deletions.
diff --git a/lib/puppet/provider/package/appdmg.rb b/lib/puppet/provider/package/appdmg.rb
@@ -55,18 +55,19 @@ def self.installpkgdmg(source, name)
     require 'open-uri'
     require 'facter/util/plist'
     cached_source = source
-    if %r{\A[A-Za-z][A-Za-z0-9+\-\.]*://} =~ cached_source
-      cached_source = "/tmp/#{name}"
-      begin
-        curl "-o", cached_source, "-C", "-", "-k", "-s", "--url", source
-        Puppet.debug "Success: curl transfered [#{name}]"
-      rescue Puppet::ExecutionFailure
-        Puppet.debug "curl did not transfer [#{name}].  Falling back to slower open-uri transfer methods."
-        cached_source = source
+    tmpdir = Dir.mktmpdir
+    begin
+      if %r{\A[A-Za-z][A-Za-z0-9+\-\.]*://} =~ cached_source
+        cached_source = File.join(tmpdir, name)
+        begin
+          curl "-o", cached_source, "-C", "-", "-k", "-L", "-s", "--url", source
+          Puppet.debug "Success: curl transfered [#{name}]"
+        rescue Puppet::ExecutionFailure
+          Puppet.debug "curl did not transfer [#{name}].  Falling back to slower open-uri transfer methods."
+          cached_source = source
+        end
       end
-    end
 
-    begin
       open(cached_source) do |dmg|
         xml_str = hdiutil "mount", "-plist", "-nobrowse", "-readonly", "-mountrandom", "/tmp", dmg.path
           ptable = Plist::parse_xml xml_str
@@ -87,8 +88,7 @@ def self.installpkgdmg(source, name)
           end
       end
     ensure
-      # JJM Remove the file if open-uri didn't already do so.
-      File.unlink(cached_source) if File.exist?(cached_source)
+      FileUtils.remove_entry_secure(tmpdir, force=true)
     end
   end
 

diff --git a/lib/puppet/provider/package/pkgdmg.rb b/lib/puppet/provider/package/pkgdmg.rb
@@ -76,18 +76,19 @@ def self.installpkgdmg(source, name)
     end
     require 'open-uri'
     cached_source = source
-    if %r{\A[A-Za-z][A-Za-z0-9+\-\.]*://} =~ cached_source
-      cached_source = "/tmp/#{name}"
-      begin
-        curl "-o", cached_source, "-C", "-", "-k", "-s", "--url", source
-        Puppet.debug "Success: curl transfered [#{name}]"
-      rescue Puppet::ExecutionFailure
-        Puppet.debug "curl did not transfer [#{name}].  Falling back to slower open-uri transfer methods."
-        cached_source = source
+    tmpdir = Dir.mktmpdir
+    begin
+      if %r{\A[A-Za-z][A-Za-z0-9+\-\.]*://} =~ cached_source
+        cached_source = File.join(tmpdir, name)
+        begin
+          curl "-o", cached_source, "-C", "-", "-k", "-L", "-s", "--url", source
+          Puppet.debug "Success: curl transfered [#{name}]"
+        rescue Puppet::ExecutionFailure
+          Puppet.debug "curl did not transfer [#{name}].  Falling back to slower open-uri transfer methods."
+          cached_source = source
+        end
       end
-    end
 
-    begin
       if source =~ /\.dmg$/i
         File.open(cached_source) do |dmg|
           xml_str = hdiutil "mount", "-plist", "-nobrowse", "-readonly", "-noidme", "-mountrandom", "/tmp", dmg.path
@@ -116,8 +117,7 @@ def self.installpkgdmg(source, name)
         raise Puppet::Error.new("Mac OS X PKG DMG's must specificy a source string ending in .dmg or flat .pkg file")
       end
     ensure
-      # JJM Remove the file if open-uri didn't already do so.
-      File.unlink(cached_source) if File.exist?(cached_source)
+      FileUtils.remove_entry_secure(tmpdir, force=true)
     end
   end