(PUP-9312) Whitelist registry reads for Windows package providers

lib/puppet/provider/package/windows/exe_package.rb

@@ -4,6 +4,19 @@ class Puppet::Provider::Package::Windows
   class ExePackage < Puppet::Provider::Package::Windows::Package
     attr_reader :uninstall_string
 
+    # registry values to load under each product entry in
+    # HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall
+    # for this provider
+    REG_VALUE_NAMES = [
+      'DisplayVersion',
+      'UninstallString',
+      'ParentKeyName',
+      'Security Update',
+      'Update Rollup',
+      'Hotfix',
+      'WindowsInstaller',
+    ]
+
     # Return an instance of the package from the registry, or nil
     def self.from_registry(name, values)
       if valid?(name, values)

lib/puppet/provider/package/windows/msi_package.rb

@@ -8,6 +8,14 @@ class MsiPackage < Puppet::Provider::Package::Windows::Package
     INSTALLSTATE_DEFAULT = 5 # product is installed for the current user
     INSTALLUILEVEL_NONE  = 2 # completely silent installation
 
+    # registry values to load under each product entry in
+    # HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\Uninstall
+    # for this provider
+    REG_VALUE_NAMES = [
+      'DisplayVersion',
+      'WindowsInstaller'
+    ]
+
     # Get the COM installer object, it's in a separate method for testing
     def self.installer
       # REMIND: when does the COM release happen?

lib/puppet/provider/package/windows/package.rb

@@ -11,6 +11,14 @@ class Package
 
     attr_reader :name, :version
 
+    REG_DISPLAY_VALUE_NAMES = [ 'DisplayName', 'QuietDisplayName' ]
+
+    def self.reg_value_names_to_load
+      REG_DISPLAY_VALUE_NAMES |
+      MsiPackage::REG_VALUE_NAMES |
+      ExePackage::REG_VALUE_NAMES
+    end
+
     # Enumerate each package. The appropriate package subclass
     # will be yielded.
     def self.each(&block)
@@ -37,7 +45,7 @@ def self.with_key(&block)
             open(hive, 'Software\Microsoft\Windows\CurrentVersion\Uninstall', mode) do |uninstall|
               each_key(uninstall) do |name, wtime|
                 open(hive, "#{uninstall.keyname}\\#{name}", mode) do |key|
-                  yield key, values(key)
+                  yield key, values_by_name(key, reg_value_names_to_load)
                 end
               end
             end

lib/puppet/util/windows/registry.rb

@@ -65,6 +65,28 @@ def values(key)
       vals
     end
 
+    # Retrieve a set of values from a registry key given their names
+    # Value names listed but not found in the registry will not be added to the
+    # resultant Hashtable
+    #
+    # @param key [RegistryKey] An open handle to a Registry Key
+    # @param names [String[]] An array of names of registry values to return if they exist
+    # @return [Hashtable<String, Object>] A hashtable of all of the found values in the registry key
+    def values_by_name(key, names)
+      vals = {}
+      names.each do |name|
+        FFI::Pointer.from_string_to_wide_string(name) do |subkeyname_ptr|
+          begin
+            _, vals[name] = read(key, subkeyname_ptr)
+          rescue Puppet::Util::Windows::Error => e
+            # ignore missing names, but raise other errors
+            raise e unless e.code == Puppet::Util::Windows::Error::ERROR_FILE_NOT_FOUND
+          end
+        end
+      end
+      vals
+    end
+
     def each_value(key, &block)
       index = 0
       subkey = nil
@@ -104,7 +126,7 @@ def reg_enum_key(key, index, max_key_length = Win32::Registry::Constants::MAX_KE
 
             if result != FFI::ERROR_SUCCESS
               msg = _("Failed to enumerate %{key} registry keys at index %{index}") % { key: key.keyname, index: index }
-              raise Puppet::Util::Windows::Error.new(msg)
+              raise Puppet::Util::Windows::Error.new(msg, result)
             end
 
             filetime = FFI::WIN32::FILETIME.new(filetime_ptr)
@@ -135,7 +157,7 @@ def reg_enum_value(key, index, max_value_length = Win32::Registry::Constants::MA
 
           if result != FFI::ERROR_SUCCESS
             msg = _("Failed to enumerate %{key} registry values at index %{index}") % { key: key.keyname, index: index }
-            raise Puppet::Util::Windows::Error.new(msg)
+            raise Puppet::Util::Windows::Error.new(msg, result)
           end
 
           subkey_length = subkey_length_ptr.read_dword
@@ -165,7 +187,7 @@ def reg_query_info_key_max_lengths(key)
 
           if status != FFI::ERROR_SUCCESS
             msg = _("Failed to query registry %{key} for sizes") % { key: key.keyname }
-            raise Puppet::Util::Windows::Error.new(msg)
+            raise Puppet::Util::Windows::Error.new(msg, status)
           end
 
           result = [
@@ -247,8 +269,10 @@ def query_value_ex(key, name_ptr, &block)
               buffer_ptr, length_ptr)
 
             if result != FFI::ERROR_SUCCESS
-              msg = _("Failed to read registry value %{value} at %{key}") % { value: name_ptr.read_wide_string, key: key.keyname }
-              raise Puppet::Util::Windows::Error.new(msg)
+              # buffer is raw bytes, *not* chars - less a NULL terminator
+              name_length = (name_ptr.size / FFI.type_size(:wchar)) - 1 if name_ptr.size > 0
+              msg = _("Failed to read registry value %{value} at %{key}") % { value: name_ptr.read_wide_string(name_length), key: key.keyname }
+              raise Puppet::Util::Windows::Error.new(msg, result)
             end
 
             # allows caller to use FFI MemoryPointer helpers to read / shape

spec/integration/util/windows/registry_spec.rb

@@ -253,5 +253,44 @@ def expects_registry_value(array)
       end
     end
   end
+
+  context "#values_by_name" do
+    let(:hkey)   { stub 'hklm' }
+    let(:subkey) { stub 'subkey' }
+
+    before :each do
+      subject.stubs(:root).returns(hkey)
+    end
+
+    context "when reading values" do
+      let (:hklm) { Win32::Registry::HKEY_LOCAL_MACHINE }
+      let (:puppet_key) { "SOFTWARE\\Puppet Labs"}
+      let (:subkey_name) { "PuppetRegistryTest#{SecureRandom.uuid}" }
+
+      before(:each) do
+        hklm.create("#{puppet_key}\\#{subkey_name}", Win32::Registry::KEY_ALL_ACCESS) do |reg|
+          reg.write('valuename1', Win32::Registry::REG_SZ, 'value1')
+          reg.write('valuename2', Win32::Registry::REG_SZ, 'value2')
+        end
+      end
+
+      after(:each) do
+        hklm.open(puppet_key, Win32::Registry::KEY_ALL_ACCESS) do |reg|
+          subject.delete_key(reg, subkey_name)
+        end
+      end
+
+      it "should return only the values for the names specified" do
+        hklm.open("#{puppet_key}\\#{subkey_name}", Win32::Registry::KEY_ALL_ACCESS) do |reg_key|
+          vals = subject.values_by_name(reg_key, ['valuename1', 'missingname'])
+
+          expect(vals).to have_key('valuename1')
+          expect(vals).to_not have_key('valuename2')
+          expect(vals['valuename1']).to eq('value1')
+          expect(vals['missingname']).to be_nil
+        end
+      end
+    end
+  end
 end
 end