metasploit-framework
metasploit-framework copied to clipboard
Fix incorrect definitions for ldap_search functions in def_wldap32.rb
This fixes up some incorrect definitions within def_wldap32.rb so it can work with x64 Windows. Note that I'm not 100% sure if these definitions are correct or not so some verification might be needed.
Also note ldap_search_sA is presently used by the lib/msf/core/post/windows/ldap.rb library via the query_ldap function, which is in turned used by the get_default_naming_context and query functions.
Therefore, this might need some verification to make sure it doesn't break anything.
Verification
- [ ] Review the PR and confirm the definitions match MSDN's documentation.
- [ ] Get a Meterpreter session on a Windows x64 host.
- [ ] From the main console run
loadpath test/modules/to load the test modules. - [ ]
post/test/railgunshould work, all tests should pass - [ ]
post/test/railgun_reverse_lookupsshould also work with all tests passing
Note for testing might be an idea to use a module such as modules/post/windows/gather/enum_ad_users.rb and check that is still working.
Found a few other functions with incorrect return definitions. Attempted to update them but not 100% sure if the definitions are correct. Let me know if you see anything that looks odd. Rebased to fix the merge conflict.
I'm still having a real hard time testing. I can't seem to get enum_ad_users to run. It's failing as it's trying to bind, and I don't know what's wrong. I'm wondering if the domain parameter to ldap_sslinitA is in the incorrect format because the docs say it's supposed to be a hostname, but the module says it's a DN.
Either way I can't seem to get it to work on my 2019 DC as either system or running as a domain admin.
Additionally, before I got to this point, I had to make some updates to mark PULONG_PTR and PCHAR as supported types for return values. I don't know if this code works, I'm guessing it needs changes but it at least allowed the module to load and start running.
diff --git a/lib/rex/post/meterpreter/extensions/stdapi/railgun/library.rb b/lib/rex/post/meterpreter/extensions/stdapi/railgun/library.rb
index d06ee4b73a..0bb1da763f 100644
--- a/lib/rex/post/meterpreter/extensions/stdapi/railgun/library.rb
+++ b/lib/rex/post/meterpreter/extensions/stdapi/railgun/library.rb
@@ -300,7 +300,7 @@ class Library
# process return value
case function.return_type
- when 'LPVOID', 'ULONG_PTR'
+ when 'LPVOID', 'PULONG_PTR', 'ULONG_PTR'
if arch == ARCH_X64
return_hash['return'] = rec_return_value
else
@@ -316,6 +316,10 @@ class Library
return_hash['return'] = (rec_return_value != 0)
when 'VOID'
return_hash['return'] = nil
+ when 'PCHAR'
+ return_hash['return'] = read_string(rec_return_value)
+ when 'PWCHAR'
+ return_hash['return'] = read_wstring(rec_return_value)
else
raise "unexpected return type: #{function.return_type}"
end
diff --git a/lib/rex/post/meterpreter/extensions/stdapi/railgun/library_function.rb b/lib/rex/post/meterpreter/extensions/stdapi/railgun/library_function.rb
index 15b2ec932b..53ee561685 100644
--- a/lib/rex/post/meterpreter/extensions/stdapi/railgun/library_function.rb
+++ b/lib/rex/post/meterpreter/extensions/stdapi/railgun/library_function.rb
@@ -43,9 +43,9 @@ class LibraryFunction
'LPVOID' => ['in', 'return'], # sf: for specifying a memory address (e.g. VirtualAlloc/HeapAlloc/...) where we don't want to back it up with actual mem ala PBLOB
'ULONG_PTR' => ['in', 'return'],
'PDWORD' => ['in', 'out', 'inout'], # todo: support for functions that return pointers to strings
- 'PULONG_PTR' => ['in', 'out', 'inout'],
- 'PWCHAR' => ['in', 'out', 'inout'],
- 'PCHAR' => ['in', 'out', 'inout'],
+ 'PULONG_PTR' => ['in', 'out', 'inout', 'return'],
+ 'PWCHAR' => ['in', 'out', 'inout', 'return'],
+ 'PCHAR' => ['in', 'out', 'inout', 'return'],
'PBLOB' => ['in', 'out', 'inout'],
}.freeze
I'm pretty confident we'll need something to that effect so those return types can work. The calls to read_string are what I'm unsure about, but I can't get the module to run to that point for testing. ~~Do you have any ideas?~~
Edit: This was the bind culprit. The type for ld needs to be LPVOID. It should also probably be consistently defined that way instead of using PHANDLE sometimes as it is for ldap_search_sA.
Looks like setting the return type to LPVOID as you suggested didn't work, but ULONG_PTR or PULONG_PTR works, likely as the former isn't treated as a pointer type.
sigh So I found that we are calling a function in RailGun with the wrong definition, and not only that but we were also calling it incorrectly. So yeah, I don't really know how that was working if it ever worked at all but there you go 🤷 ldap_set_option should have its last parameter be a pointer to a parameter value but instead we are just passing it the parameter itself directly.
Seems like DWORD parameters are AND'd with 0xffffffff before being packed in their native architecture, presumably to truncate it down to a 32 bit number. I'm not sure why this action is making ldap_set_option work but it is. As for where this value is retrieved it seems to be from updated return value of ldap_sslinitA.
Alright since this is now kind of in an odd place lets break this down by the functions defined and which ones have been tested vs not tested so we know where this PR stands.
Functions
Used
- [x] ldap_unbind
- [x] ldap_sslinitA
- [x] ldap_bind_sA
- [x] ldap_search_sA
- [x] ldap_set_option
- [x] ldap_count_entries
- [x] ldap_first_entry
- [x] ldap_msgfree
Unused
- [ ] ldap_search_ext_sA
- [ ] ldap_next_entry
- [ ] ldap_first_attributeA
- [ ] ldap_next_attributeA
- [ ] ldap_count_values
- [ ] ldap_get_values
- [ ] ldap_value_free
- [ ] ldap_memfree
- [ ] ber_free
- [ ] ldap_err2string
@smcintyre-r7 Reviewed all used functions that have been modified and confirmed they work on a x64 target. Is there any additional work that needs to be done for this to be landed?
Per Slack convo, squashing these commits down before we go ahead and land this.
Release Notes
This fixes some incorrect Railgun definitions for the wldap32 Windows library.