Pass pointer to LsaFreeMemory, not FFI::MemoryPointerbtm/13-fix-lsa-heap-corruption

LsaFreeMemory takes a pointer to the struct in memory created by LsaEnumerateAccountRights.
We have to pass it the actual pointer, not the FFI::MemoryPointer object.

This fixes masked access violations which occasionally lead to a heap corruption which
presents itself as silent termination of chef-client/ruby with a return code of -1073740940
or STATUS_HEAP_CORRUPTION / 0xc0000374.

Fixes #6589

Signed-off-by: Bryan McLellan <btm@loftninjas.org>

1 files changed, 14 insertions, 16 deletions

diff --git a/lib/chef/win32/security.rb b/lib/chef/win32/security.rb
index c7d3f55a40..a58fa48399 100644
--- a/lib/chef/win32/security.rb
+++ b/lib/chef/win32/security.rb
@@ -113,10 +113,7 @@ class Chef
 
         with_lsa_policy(name) do |policy_handle, sid|
           result = LsaAddAccountRights(policy_handle.read_pointer, sid, privilege_pointer, 1)
-          win32_error = LsaNtStatusToWinError(result)
-          if win32_error != 0
-            Chef::ReservedNames::Win32::Error.raise!(nil, win32_error)
-          end
+          test_and_raise_lsa_nt_status(result)
         end
       end
 
@@ -190,15 +187,14 @@ class Chef
           result = LsaEnumerateAccountRights(policy_handle.read_pointer, sid, privilege_pointer, privilege_length)
           win32_error = LsaNtStatusToWinError(result)
           return [] if win32_error == 2 # FILE_NOT_FOUND - No rights assigned
-          if win32_error != 0
-            Chef::ReservedNames::Win32::Error.raise!(nil, win32_error)
-          end
+          test_and_raise_lsa_nt_status(result)
 
           privilege_length.read_ulong.times do |i|
             privilege = LSA_UNICODE_STRING.new(privilege_pointer.read_pointer + i * LSA_UNICODE_STRING.size)
             privileges << privilege[:Buffer].read_wstring
           end
-          LsaFreeMemory(privilege_pointer)
+          result = LsaFreeMemory(privilege_pointer.read_pointer)
+          test_and_raise_lsa_nt_status(result)
         end
 
         privileges
@@ -595,18 +591,13 @@ class Chef
 
         policy_handle = FFI::MemoryPointer.new(:pointer)
         result = LsaOpenPolicy(nil, LSA_OBJECT_ATTRIBUTES.new, access, policy_handle)
-        win32_error = LsaNtStatusToWinError(result)
-        if win32_error != 0
-          Chef::ReservedNames::Win32::Error.raise!(nil, win32_error)
-        end
+        test_and_raise_lsa_nt_status(result)
 
         begin
           yield policy_handle, sid.pointer
         ensure
-          win32_error = LsaNtStatusToWinError(LsaClose(policy_handle.read_pointer))
-          if win32_error != 0
-            Chef::ReservedNames::Win32::Error.raise!(nil, win32_error)
-          end
+          result = LsaClose(policy_handle.read_pointer)
+          test_and_raise_lsa_nt_status(result)
         end
       end
 
@@ -654,6 +645,13 @@ class Chef
         end
         Token.new(Handle.new(token.read_pointer))
       end
+
+      def test_and_raise_lsa_nt_status(result)
+        win32_error = LsaNtStatusToWinError(result)
+        if win32_error != 0
+          Chef::ReservedNames::Win32::Error.raise!(nil, win32_error)
+        end
+      end
     end
   end
 end