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
Add RDI mixin module and refactor things to use it #2721
Conversation
MSF was starting to see more modules using RDI to load binaries into remote processes, so it made sense to create a mixin which contained the functionality that was being used in various locations. This commit contains the new mixin, and adjustments to all the existing exploits and modules which use RDI.
# | ||
### | ||
|
||
module Msf::RdiMixin |
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.
The 'mixin' bit is redundant? I think this should just be Msf::ReflectiveDLLInjection, or Msf::RDI, or Msf::RDLI or Msf::RDLInjection.
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.
While I agree, I was copying the an existing mixin, such as PostMixin
.
Changed `RdiMixin` to `ReflectiveDLLInjection`.
@@ -1,7 +1,7 @@ | |||
# -*- coding: binary -*- | |||
|
|||
require 'msf/core' | |||
require 'rex/peparsey' | |||
require 'msf/core/rdi_mixin' |
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 would suggest renaming the file as well
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.
to ... msf/core/reflecivedllinjection_mixin.rb
? :)
Everything working here, my only reservation is whether it should be split into two modules. The current module keeps and an I'm not sure about inject_into_process I think this should maybe be moved into Msf::Post::Windows::Process which already has Maybe just move the |
Oops wrong button |
# Load a reflectively-injectable DLL from disk and find the offset | ||
# to the ReflectiveLoader function inside the DLL. | ||
# | ||
# +dll_path+ - Path to the DLL to load. |
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.
Yardoc standards, please.
# @param dll_path [String] Path to the DLL to load.
#
# @return [Array] Tuple of ...
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.
My bad. I took a shot in the dark rather than proper research here. Will fix, thank you!
|
But its used by payloads so shouldn't be under post... |
What @Meatballs1 said. I didn't really think that it belonged there thanks to the RDI payloads using it. Is there anywhere better to put it? |
Herein lies the problem, as it's still an injection concern, not just a process concern. So is it worth it? I kind of like the "one place" for RDI related things. |
I was thinking that Msf::Post::Windows::RDI then depends upon Msf::Post::Windows::Process and Msf::RDI. You cant RDI without a process or the process functions (well you could railgun them). nb RDI is just shorthand I think it looks too ugly as a module name. |
Agreed, I'm using the full name in modules. So I'm splitting it into two:
I'll take a look at the |
Now broken into two modules, one for loading RDI DLLs off disk and finding the loader function offset, and another for doing the process specific stuff of loading into the target.
|
||
raise "Can't find an exported ReflectiveLoader function!" if offset == 0 | ||
dll, offset = load_rdi_dll(library_path) | ||
raise "Can't find an exported ReflectiveLoader function!" unless offset |
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.
Redundant now?
rescue ::Exception => e | ||
print_error("Failed to Inject Payload to #{pid}!") | ||
vprint_error(e.to_s) | ||
if dll_mem && offset |
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.
This if/else block should be redundant now also as we will hit exception on line 46?
Retested kitrapo0d ppr_flatten_rec and reflective_dll_injection and all still working. Just want to check with @jlee-r7 that he's happy with current file structure. |
Yup! Me too. |
print_good("Process #{pid} launched.") | ||
|
||
print_status("Reflectively injecting the exploit DLL into #{pid}...") | ||
process = client.sys.process.execute("notepad.exe", nil, {'Hidden' => true}) |
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.
Might be nice to expose the spawned process name as a datastore option, but not necessary for this PR
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.
Aye true. This could apply to a lot of other modules outside of this PR. Happy to do it after though.
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.
For that rabbit hole:
grep -r -i "notepad.exe" . | wc -l
39
# | ||
# Inject the given shellcode into a target process. | ||
# | ||
# @param process The process to inject the shellcode into. |
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.
# @param process [Rex::Post::Meterpreter::Extensions::Stdapi::Sys::Process]
# The process to inject the shellcode into.
A few style nitpicks, but otherwise this looks shiny. Great work folks. |
Done. Thanks @jlee-r7 ! @Meatballs1 said he'd merge in the morning if you were cool with it. Thanks to all for their help! First Ruby mixin for me, learned lots. Much appreciated. |
Adds support to load a dll and identify the ReflectiveLoader offset. Adds support to inject dll into process and execute it. Updates kitrap0d, ppr_flatten_rec, reflective_dll_inject modules and payload modules to use above features.
Legend @Meatballs1 ! Thank you sir. |
Rationale
MSF was starting to see more modules using RDI to load binaries into remote processes, so it made sense to create a mixin which contained the functionality that was being used in various locations.
This commit contains the new mixin, and adjustments to all the existing exploits and modules which use RDI.
I haven't done a mixin before, so I picked a spot which I thought made sense and put it there. As always, this PR is open to lots of critique to make sure we get it right.
Validation
Kitrap0d
worksppr_flatten_rec
worksreflective_dll_inject
worksTest Runs
HTTP Payload
KiTrap0D
ppr_flatten_rec
Reflective Loading x86
Reflective Loading x64