Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

(#10614) Fix error checking for Windows BOOL return values

Previously, the agent could segfault when attempting to manage a DACL
on a non-NTFS filesystem. The problem was that we were incorrectly
checking the return value from several of the Windows APIs due to the
windows gems automatically converting BOOL values into ruby true/false
values.

For example, this call returns a ruby true/false value:

  API.new('IsValidAcl', 'P', 'B', 'advapi32')

But this one will return an integer value:

  API.new('GetSecurityInfo', 'LLLPPPPP', 'L', 'advapi32')

The crash is now fixed because we correctly raise an exception when
IsValidAcl returns false.

Paired-with: Nick Lewis <nick@puppetlabs.com>
  • Loading branch information...
commit 7eb0197f33bc12cd4a18c157bd384bdf8b6d717f 1 parent c95aafe
Josh Cooper joshcooper authored
Showing with 13 additions and 12 deletions.
  1. +13 −12 lib/puppet/util/windows/security.rb
25 lib/puppet/util/windows/security.rb
View
@@ -363,7 +363,7 @@ def set_acl(path, protected = true)
raise Puppet::Util::Windows::Error.new("Failed to initialize ACL")
end
- raise Puppet::Util::Windows::Error.new("Invalid DACL") if IsValidAcl(acl) == 0
+ raise Puppet::Util::Windows::Error.new("Invalid DACL") unless IsValidAcl(acl)
yield acl
@@ -380,9 +380,9 @@ def set_acl(path, protected = true)
def add_access_allowed_ace(acl, mask, sid, inherit = NO_INHERITANCE)
string_to_sid_ptr(sid) do |sid_ptr|
- raise Puppet::Util::Windows::Error.new("Invalid SID") if IsValidSid(sid_ptr) == 0
+ raise Puppet::Util::Windows::Error.new("Invalid SID") unless IsValidSid(sid_ptr)
- if AddAccessAllowedAceEx(acl, ACL_REVISION, inherit, mask, sid_ptr) == 0
+ unless AddAccessAllowedAceEx(acl, ACL_REVISION, inherit, mask, sid_ptr)
raise Puppet::Util::Windows::Error.new("Failed to add access control entry")
end
end
@@ -390,9 +390,9 @@ def add_access_allowed_ace(acl, mask, sid, inherit = NO_INHERITANCE)
def add_access_denied_ace(acl, mask, sid)
string_to_sid_ptr(sid) do |sid_ptr|
- raise Puppet::Util::Windows::Error.new("Invalid SID") if IsValidSid(sid_ptr) == 0
+ raise Puppet::Util::Windows::Error.new("Invalid SID") unless IsValidSid(sid_ptr)
- if AddAccessDeniedAce(acl, ACL_REVISION, mask, sid_ptr) == 0
+ unless AddAccessDeniedAce(acl, ACL_REVISION, mask, sid_ptr)
raise Puppet::Util::Windows::Error.new("Failed to add access control entry")
end
end
@@ -401,7 +401,7 @@ def add_access_denied_ace(acl, mask, sid)
def get_dacl(handle)
get_dacl_ptr(handle) do |dacl_ptr|
# REMIND: need to handle NULL DACL
- raise Puppet::Util::Windows::Error.new("Invalid DACL") if IsValidAcl(dacl_ptr) == 0
+ raise Puppet::Util::Windows::Error.new("Invalid DACL") unless IsValidAcl(dacl_ptr)
# ACL structure, size and count are the important parts. The
# size includes both the ACL structure and all the ACEs.
@@ -422,7 +422,8 @@ def get_dacl(handle)
0.upto(ace_count - 1) do |i|
ace_ptr = [0].pack('L')
- next if GetAce(dacl_ptr, i, ace_ptr) == 0
+
+ next unless GetAce(dacl_ptr, i, ace_ptr)
# ACE structures vary depending on the type. All structures
# begin with an ACE header, which specifies the type, flags
@@ -524,9 +525,9 @@ def sid_ptr_to_string(psid)
sid_buf = 0.chr * 256
str_ptr = 0.chr * 4
- raise Puppet::Util::Windows::Error.new("Invalid SID") if IsValidSid(psid) == 0
+ raise Puppet::Util::Windows::Error.new("Invalid SID") unless IsValidSid(psid)
- raise Puppet::Util::Windows::Error.new("Failed to convert binary SID") if ConvertSidToStringSid(psid, str_ptr) == 0
+ raise Puppet::Util::Windows::Error.new("Failed to convert binary SID") unless ConvertSidToStringSid(psid, str_ptr)
begin
strncpy(sid_buf, str_ptr.unpack('L')[0], sid_buf.size - 1)
@@ -594,14 +595,14 @@ def set_privilege(privilege, enable)
tmpLuid = 0.chr * 8
# Get the LUID for specified privilege.
- if LookupPrivilegeValue("", privilege, tmpLuid) == 0
+ unless LookupPrivilegeValue("", privilege, tmpLuid)
raise Puppet::Util::Windows::Error.new("Failed to lookup privilege")
end
# DWORD + [LUID + DWORD]
tkp = [1].pack('L') + tmpLuid + [enable ? SE_PRIVILEGE_ENABLED : 0].pack('L')
- if AdjustTokenPrivileges(token, 0, tkp, tkp.length , nil, nil) == 0
+ unless AdjustTokenPrivileges(token, 0, tkp, tkp.length , nil, nil)
raise Puppet::Util::Windows::Error.new("Failed to adjust process privileges")
end
end
@@ -611,7 +612,7 @@ def set_privilege(privilege, enable)
def with_process_token(access)
token = 0.chr * 4
- if OpenProcessToken(GetCurrentProcess(), access, token) == 0
+ unless OpenProcessToken(GetCurrentProcess(), access, token)
raise Puppet::Util::Windows::Error.new("Failed to open process token")
end
begin
Please sign in to comment.
Something went wrong with that request. Please try again.