New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow .NET assembly execution within the meterpreter process #18114
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a few comments from my first review but I'm not done yet and I haven't tested things yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left the rest of my comments from my review now that I've finished. Also the PPID fix has been landed so would you mind rebasing this to pull in the changes from #18129 which bumps the metasploit-payloads gem to include it?
pipe_suffix = Rex::Text.rand_text_alphanumeric(8) | ||
pipe_name = "\\\\.\\pipe\\#{pipe_suffix}" | ||
appdomain_name = Rex::Text.rand_text_alpha(9) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you mind please vprinting these values? They could be useful in the future if we need to debug something.
# This has to be here, not in cleanup, because if an exception is raised | ||
# on the MSF side, we don't want to prematurely clean up the DLL that may | ||
# still be running. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you update the C++ code to exit by calling ReflectiveFreeAndExitThread
I think you can not only avoid this issue but also free the memory that the loader itself allocates here.
When I added that code in rapid7/metasploit-payloads#631, I tested it pretty thoroughly on a few different versions of Windows including x86 and x64.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean to copy that function into the HostingCLR solution and call it from there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Until we move it into a central location in the ReflectiveDLLInjection repository where it probably belongs, we'll need to copy it around. AFAIK, this will be the second time it's used so the first time it's been copied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done that, and am now freeing the three chunks of memory (assembly+params, reflectively loaded DLL and bootstrapping code) in the C++ side.
f1a1a9b
to
abf5ab4
Compare
…g our own process) and receive output.
abf5ab4
to
6e438d3
Compare
9da2d4f
to
a7ce4c7
Compare
The _AppDomainPtr, _AssemblyPtr and _MethodInfoPtr variables are COM smart pointers which will auto-Release() when they go out of scope, so we should not directly Release() them.
Below you'll find the details of the issues I found while testing this. Unless otherwise stated, the target was a Windows Server 2019 x64 instance that has been promoted to being a DC.
|
I did some testing on a 64-bit Windows 2016 (10.0 Build 14393) target and found the issue below. Fails when attempting to copy the assemblySteps to reproduce:
|
Fixed a crash and broken logic in hosting clr code.
Thanks for the testing @smcintyre-r7. I've fixed up those stack traces with friendlier errors - I think in all cases, the errors make sense (i.e. nothing to fix per se), but it certainly was messy. The crash you described when doing spawn-and-inject then self then spawn-in-inject was quite a bit of fun to trace down. It turned out to be a bug related to the ETW evasion: I hadn't realised that the solution overrode the There are a few ways to bypass ETW, so I think the easiest solution here is just to patch it to return immediately (rather than inserting the intercepted function). |
Thanks for looking into it @jheysel-r7 - I'm unable to reproduce that error - using the same OS build and Seatbelt build. Does it reliably crash for you? |
Hey @smashery, I'm sorry you couldn't reproduce the error. I am unfortunately seeing it reliably crash for me. I've tried using different sessions on my MacOS and Ubuntu dev machine. All testing was conducted against a Windows Server 2016 with the payload: msfconsole host: MacOS 13.4.1, session user NT AUTHORITY\SYSTEM
msfconsole host: Ubuntu 20.04, session user WIN-UUA4G232GU1\msfuser
|
Tested on a Windows 2019 Server that has been upgraded to a DC, which has less versions of dotnet installed on it (compared to my 2016 target) and I did not see the same issue as above, the module ran as expected: Successful Windows 2019 Server output
|
I tried removing the dotnet 3.5 feature from my Windows 2016 target (to match the dotnet versions that my 2019 server had when the module ran successfully) however that did not help. Still seeing the same failure. Apologies for the test spam, let me know if there's any other tests I can run to help. Same failure on Windows 2016 with only ["4.7.02053", "4.0.0.0"] dot net versions installed
|
Oh good catch - curious as to why Windows is fine with it sometimes and not others, but 🤷 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With that last commit, the issue we were running into on Server 2016 has been resolved. Everything looks ready to me now so I'll get this landed. Thanks @smashery
Allow .NET assembly execution within the meterpreter process
Release NotesThis updates the |
This modifies the
execute_dotnet_assembly
module in a few ways; primarily supporting the ability to run the module within meterpreter itself, rather than the existing behaviour of requiring execution in a separate process.Supporting this work has meant a few significant changes:
Known issues
There are a few situations which are known to not work; but these existed in the module prior to these changes:
Technical details of the changes
So the big changes here:
Verification steps
execute_dotnet_assembly
with a range of assemblies under various OSes and .NET versions.