-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
Fix empty cmd/windows/powershell/download_exec payload #18609
Fix empty cmd/windows/powershell/download_exec payload #18609
Conversation
@@ -644,7 +645,7 @@ def build(asm, off={}) | |||
end | |||
|
|||
# Assemble the payload from the assembly | |||
a = self.arch | |||
a = opts[:arch] || self.arch |
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.
It looks like the original adapted arch is passed in from opts
, so this is:
[1] pry(#<#<Class:0x00007fab4c96f308>>)> opts
=> {:arch=>"x86"}
But I also had this line as this previously, similar to the pattern over here
a = module_info["AdaptedArch"] || self.arch
Does anyone have any preferences on the chosen approach here?
From a quick look, it seems like there's other references in this file too that use self.arch
, which might also be wrong
The other places (compatible_encoders
and compatible_nops
) seem like they're (probably) fine, as it feels like it should be using cmd
as the arch
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 think the way it is currently is correct. self.arch
is the architecture of the over all payload which would be ARCH_CMD in the case of cmd/windows/powershell/download_exec
but at this point, it needs to be ARCH_X86 as specified in the options from the caller. This is the pattern I took in #16597. 👍
Release NotesThis fixes an issue in the |
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 was able to reproduce the original issue and validate that the proposed changes fix it 👍
Fix a bug in
cmd/windows/powershell/download_exec
which caused empty payloads to be generatedCloses #18607
Debugging Notes
Adding this debug line:
Shows the unadapted payload works:
But the adapter doesn't:
This is because the adapted payload arch is identified as
["cmd"]
, so it doesn't convert the ASM payload correctly:Verification
Verify that the steps in #18607 work