Closed sempervictus closed 7 years ago
@sempervictus Any chance you can copy over background notes from the original PR to this one?
I've probably had several major strokes/epiphanies since writing this stuff. The meat of what this does can be seen at the commit itself.
For reference: Original PR was #701 (time flies).
I think this showed up in a few others, but thats the earliest one i found.
It looks like the some powershell rspecs need to be updated. Please see: https://travis-ci.org/rapid7/metasploit-framework/jobs/64005267
@sempervictus It looks like there's some problem w/ the code. Specifically when rspec does this:
subject.cmd_psh_payload(payload, arch, {:encode_final_payload => true, :no_equals => false})
And
subject.cmd_psh_payload(payload, arch, {:encode_final_payload => true, :no_equals => true})
The cmd_psh_payload method just returns an empty string. Do you remember if this the correct behavior? If it does return an empty string, I feel like it should raise.
I should have caught this in review, the bug's been there almost 2 years. 2cf39d4b made some pretty significant changes and i didnt catch line 325 of lib/rex/powershell/command.rb:
command_args[:encodedcommand] = encode_script(final_payload)
Which the command parser doesn't process properly, it's looking for :command. Lemme dig into this a minute and figure out whether i need to fix that line or the command processor
sweet, thank you!
Strangeness:
OptBool.new('Powershell::exec_in_place', [true, 'Produce PSH without executable wrapper', false]),
Should be a boolean value, but looking at the opts being passed around in generate_pshcommand i'm seeing :exec_inplace=>"false" which is triggering
psh_command = opts[:exec_in_place] ? "#{command_args[:command]}" : generate_psh_command_line(command_arg
to return command_args[:command] when the data is in :encodedcommand.
I've pushed a commit to fix this by forcing the command_args[:exec_in_place] to false when we hit
if opts[:encode_final_payload]
and assigning the blob to the :command key for display.
While this fixes the bug you spotted, it still concerns me that we're getting improper options passed around in here. The call chains can get recursive, weird, and generate a ton of extra encoding wrappers we dont need if we're not careful.
Ok, the spec failure indicates that what i did was wrong. Meatballs seems to have intended for this to be prefaced with powershell -e. I'm a bit concerned that i'm mucking about in his work without the proper blessing. Ping @Meatballs1, i think the actual issue diagnosed properly, but i'd like your take on what the deuce could be causing this behavior
Hmm. Another failure, different issue. Digging.
Same bug, couple of lines down. Looking to find the anthill.
Somehow, when this method is called outside of a module in direct Ruby, the data type in the DS are not respected. lib/msf/core/exploit/powershell.rb assigns options from the DS, and they work fine when called from the normal context, but apparently we have a breakdown somewhere which is processing the DS as a hash, and when manually calling this method from MSF (Rex seems ok), we're assigning strings to all the other data types.
Either i'm dense enough to sink through granite today, or we have breakage in the core somewhere.
I have seen this behavior before. I left some notes here describing my understanding of it: https://github.com/rapid7/metasploit-framework/issues/6287
If I remember correctly, basically, the core loads the module by using:
framework.modules.create(mod_name)
And then it never tries to normalize. But on the module's level, at some point it does before it's run.
Yeah, this is not the first time i've seen something of this nature. If we're marshalling modules around with datastores, we should ensure that our datastore actually contains objects of the appropriate type (three cheers for dynamic typing and bullets turning to gummy bears mid air). The source of the problem is a bit outside the scope of the PR, but i'm game to try and tackle this with some input from the core team. I need to do some more reading before i have a take on how we fix this, given that i dont know if anything actually depends on the #to_s behavior seen here. @devs: thoughts?
So something along the lines of
datastore.keys.map do |k|
if options[k].respond_to?(:normalize)
datastore[k] = options[k].normalize(datastore[k])
end
end
might be useful as a starting point for a callback which would ensure the DS has proper types, but still not sure where we'd need to inject this.
Interesting, looks like some of the inner PSH encoding/evasion features weren't being exposed to the Msf tier properly. Fixed.
Handled the datastore issue by forcing a validation call from options at the start of cmd_psh_payload. This should still happen automatically IMO, but at least it should address the problems seen in review.
Rebased on master (clean), forced push, eagerly awaiting the next salvo of Travis' profanity.
/me throws rock at travis. That's confusing:
1) Msf::Exploit::Powershell::cmd_psh_payload when method is unknown should raise an exception
Which it does, so i'm not sure why that's causing a failure. I'd bet it has something to do with the OptEnum designation. More rats in this nest. @hdm: If you're still watching this thread, mind adding your $0.02 on the proper/most elegant way to address the data type problem?
1) Msf::Exploit::Powershell::cmd_psh_payload when method is unknown should raise an exception
Failure/Error: subject.cmd_psh_payload(payload, arch)
Msf::OptionValidateError:
The following options failed to validate: Powershell::method.
# ./lib/msf/core/exploit/powershell.rb:195:in `cmd_psh_payload'
# ./spec/lib/msf/core/exploit/powershell_spec.rb:332:in `block (4 levels) in <top (required)>'
The OptEnum
prevents submitting :blah
so causes a Msf:OptionValidateError
to be raised, but we are testing for a RuntimeError
I suggest the test is updated to catch either of these exceptions... And maybe I should be more specific when I say 'should raise an exception' to say 'should raise a RuntimeException'
context 'when method is unknown' do
it 'should raise an exception' do
except = false
begin
subject.cmd_psh_payload(payload, arch, template_path, method: 'blah')
rescue RuntimeError
except = true
end
expect(except).to be_truthy
end
end
Additionally I think in current rspec you can test for exceptions in a proper fashion e.g:
expect { subject.cmd_psh_payload(payload, arch, template_path, method: 'blah')}.to raise_error(RuntimeError, 'No Powershell method specified')
But not sure how you could rescue both Runtime and Msf:OptionValidationError without just grabbing all exceptions.
But not sure how you could rescue both Runtime and Msf:OptionValidationError without just grabbing all exceptions.
Like this?
rescue Runtime, Msf:OptionValidationError => e
if /something/ === e.message
# handling something specific
else
raise e
end
Yeah but thats not very rspeccy :D
oh rspec
Thanks for diving in Meatballs1, and the clarification. While I DO want to catch the spec scope problems, I'm curious to get your take on the root cause here. That options.validate(datastore) fix feels like a gorilla patch. On Feb 1, 2016 10:37, "sinn3r" notifications@github.com wrote:
oh rspec
— Reply to this email directly or view it on GitHub https://github.com/rapid7/metasploit-framework/pull/5393#issuecomment-178026041 .
Should probably see if we can call options.validate
from the rspec rather than the code if it is just the rspec failing.
Well, thats the thing, its not just the spec failing. I was able to walk this in pry from the psexec module since it imports all the right namespaces. Given that i've seen other representations of this issue over the years, i think we may have some deeply nested framework bug in the way we deal with DS datatypes which has been masked by Ruby and some neat coding which deals with expressions of this bug when parameters are instantiated (like how we create sockets from params). Worst part is that its likely that there are parts of framework which were written atop the behavior, depending on the String representations of other data types in the DS. To see what i mean, set up a psexec module, pry it, hit options.validate(datastore) and then exploit and see how it behaves. This seems exactly the sort of thing Crystal and similar languages are trying to avoid. Wonder if we should enforce type-checking callbacks in our objects after each method invocation (monitor threads/fibers or similar?). Would be interesting to see how that'd hit the GCs in MRI and RBX too...
This is definitely present throughout framework, as i'm seeing symptoms in modules now - take the Nagios NRPE command execution exploit for instance, if you try to #check, it won't actually try the TLS connection since it checks a datastore key for presence of any value not the type or value thereof (FalseClass). Pretty much anywhere we have code a la
do_stuff if datastore[key]
on an OptBool, we're liable to hit this bug. Probably best to fix in Framework than to ask the community to stop using Rubyisms to express themselves.
Are you certain about that issue with OptBool? It seems to work as expected in master at least.
Try running the spec with the first two commits only, or just pry-ing a module with the failing options we discussed above. I'm hoping you're right and i'm wrong, because the "cleanest" way i see to solve this right now is forcing the datastore or the context's option set to validate each set DS value with the corresponding opt in some transaction-group-type approach for every set/unset.
I made #6644 to solve the OptBool problem. While there were some places where we tried to workaround the 'false' problem, there were plenty where we were not handling it too.
This PR should be passing now when rebased on the latest upstream master
I will try this PR again tomorrow.
Is this made easier at all by the inclusion of the powershell extension @sempervictus @OJ .. Looking forward to the WMI execution post module in #6638
Hi @sempervictus could you please merge this? https://github.com/sempervictus/metasploit-framework/pull/22
Apologizes, just saw this, will do tonight when I'm back at the environment with GH access (pending review). Thank you. On Mar 31, 2016 3:52 PM, "sinn3r" notifications@github.com wrote:
Hi @sempervictus https://github.com/sempervictus could you please merge this? sempervictus#22 https://github.com/sempervictus/metasploit-framework/pull/22
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/rapid7/metasploit-framework/pull/5393#issuecomment-204099215
no problem. Thank YOU!
Hi @sempervictus, I am able to get ps_persist to work for me after a small code change. Basically the psh_exec method keeps timing out, so it looks like it needs:
begin
ps_out = psh_exec(com_script)
rescue Rex::TimeoutError
end
Not sure if that makes sense to you?
Anyways, that module works for me on Win 7 + Powershell 2.0, and Win 8.1 + Powershell 4.0.
But build_net_code does not seem to work for me at all. First off, there a few minor issues:
def exploit
doesn't work.OUTPUT_TARGET
is required, but not really. Those two are easy, so moving on...
Here's my test file:
$ cat /tmp/hello.cs
using System;
public class Hello
{
public static void Main()
{
Console.WriteLine("Hello, World!");
}
}
And then this is what I get:
msf post(build_net_code) > show options
Module options (post/windows/manage/powershell/build_net_code):
Name Current Setting Required Description
---- --------------- -------- -----------
ASSEMBLIES mscorlib.dll, System.dll, System.Xml.dll, System.Data.dll, System.Net.dll no Any assemblies outside the defaults
CODE_PROVIDER Microsoft.CSharp.CSharpCodeProvider yes Code provider to use
COMPILER_OPTS /optimize no Options to pass to compiler
OUTPUT_TARGET no Name and path of the generated binary, default random, omit extension
RUN_BINARY false no Execute the genrated binary
SESSION 1 yes The session to run this module on.
SOURCE_FILE /tmp/hello.cs yes Path to source code
msf post(build_net_code) > run
[*] Compressing script contents:
[+] - Compressed size: 372
[*] Executing the script.
[-] File C:\Users\sinn3r\AppData\Local\Temp\YCwOalZGCgGFiU.exe not found
[*] Post module execution completed
msf post(build_net_code) >
It doesn't give me much debugging info, so that's all I have for you.
Please let me know how you'd like to proceed. Thanks.
@sempervictus I'll unassign myself from the PR. When you have time to look at this again, let me know, and then I'll try again. Thanks.
@brandonprry would you be interested in helping us across the line here?
Hey, I am currently traveling, will be back home later this week. What are your concerns?
I just got back into civilization, will assign this for my evening tasks. On Jun 12, 2016 13:14, "Brandon Perry" notifications@github.com wrote:
Hey, I am currently traveling, will be back home later this week. What are your concerns?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rapid7/metasploit-framework/pull/5393#issuecomment-225448743, or mute the thread https://github.com/notifications/unsubscribe/ABRPjNv_9nvda_ylDqkUYKzffXJcQNNqks5qLD5ygaJpZM4Ei5Es .
Took a first look over this, its very broken now. Looks like a present, we're not even hitting the compiler's call chain, the run method is missing, and all sorts of things have hit the fan on this one. Gimme a couple of hours and i'll update the PR.
Works again.
Could someone on a properly upstream version of framework test the updated builder module? I ran this in my fork without the internal plugin from where it comes, and i'm getting compiled code, but i'm not sure how this interacts with the new meterpreter PSH plugin yet (i keep it disabled).
Sample output as of now:
(2016-06-12)18:16 (S:2 J:4)msf post(build_net_code) > show options
Module options (post/windows/manage/powershell/build_net_code):
Name Current Setting Required Description
---- --------------- -------- -----------
ASSEMBLIES mscorlib.dll, System.dll, System.Xml.dll, System.Data.dll no Any assemblies outside the defaults
CODE_PROVIDER Microsoft.CSharp.CSharpCodeProvider yes Code provider to use
COMPILER_OPTS /optimize no Options to pass to compiler
OUTPUT_TARGET C:\test.exe no Name and path of the generated binary, default random, omit extension
RUN_BINARY true no Execute the genrated binary
SESSION 2 yes The session to run this module on.
SOURCE_FILE /tmp/hello.cs yes Path to source code
(2016-06-12)18:22 (S:2 J:4)msf post(build_net_code) > rexploit
[*] Reloading module...
[*] [2016.06.12-18:22:43] Compressing script contents:
[*] [2016.06.12-18:22:43] Executing the script.
[+] [2016.06.12-18:23:19] File C:\test.exe found, 3072kb
[+] [2016.06.12-18:23:19] Finished!
[*] Post module execution completed
(2016-06-12)18:24 (S:2 J:4)msf post(build_net_code) > cat /tmp/hello.cs
[*] exec: cat /tmp/hello.cs
using System;
public class Hello
{
public static void Main()
{
Console.WriteLine("Hello, World!");
}
}
Resulting exe:
(2016-06-12)18:25 (S:1 J:4)msf post(build_net_code) > sessions -i 1
[*] Starting interaction with 1...
meterpreter > shell
Process 2464 created.
Channel 2 created.
Microsoft Windows [Version 6.1.7600]
Copyright (c) 2009 Microsoft Corporation. All rights reserved.
C:\Windows\system32>C:\test.exe
C:\test.exe
Hello, World!
@sempervictus yeah, I'll give it a try again. Thanks!!
Oops, this needs to be rebased with a bump in rex-powershell to be merge-ready.
Test-ready you mean :-)
Grabbing this one to rebase. Persistence is looking good here on Windows 10
dotnet compilation is a little strange when we're targeting 3.5 compatiblity. I tried this out against 4.6, and initially the script returns this on execution:
E:\metasploit-framework>powershell -executionpolicy bypass -file test.ps1
Version v4.0.30319 of the .NET Framework is not installed and it is required to run version 3 of Windows PowerShell.
I had a hunch that I needed to install dotnet 3.5, so added it. Now it fails like this instead on Windows 10:
E:\metasploit-framework>powershell -executionpolicy bypass -file test.ps1
Internal Windows PowerShell error. Loading managed Windows PowerShell failed with error 80070002.
But, there's a light at the end of the tunnel! Setting NET_CLR_VER 4.0 works like a charm:
[+] File C:\cygwin64\tmp\ebIMhzwMrOLe.exe found, 3584kb
[+] Finished!
[*] Post module execution completed
Manually closing, I rebased to land this without conflict or a giant mess of a history.
I made a few code improvements, additions, and added example documentation: 199bf8e7262dac02b5514cf1320b588815f7b9d3 5284db6b58410e4aba3a67f2f8fedca0744f6a4a df597a7bb750f833b3c0192c3ed8ed4c56345914 63bf93be1b01e76f75ef577a871dc377da322519 bd24e7eba0e27417f1af64cbc4266dd6b64e69d9
Bonus commit 7c1fa3eb5180231da3657564c44f65dae6f98b42 also fixed the invocation of 'info -d' with a module name other than the active one (noted while writing docs).
Thanks all, and any further bugs we can work out in-tree.
Looks like rebasing off of #5390 was not a great idea. Please see commit log for details, this likely supercedes the prior PR.
@Meatballs1: ping.