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

make payload generation failures at boot time non-fatal #12291

Merged
merged 2 commits into from Sep 8, 2019

Conversation

@busterb
Copy link
Member

commented Sep 7, 2019

Currently, if any payload fails to generate that has a dynamic size, it causes a Framework instance to throw an exception on start. This can happen for a number of reasons, and more often than not it is enviromental (files missing, Y2k38 bugs, etc.). Instead of failing entirely, catch the exception and log as an error, don't register the payload, but continue booting.

This fixes #12264. which was inspired by #12248, #9556 and others.

Verification

  • Modify a payload to fail in some way, e.g. apply a patch like this:
diff --git a/lib/msf/core/payload/android.rb b/lib/msf/core/payload/android.rb
index 5609480f37..193fcb0015 100644
--- a/lib/msf/core/payload/android.rb
+++ b/lib/msf/core/payload/android.rb
@@ -109,7 +109,7 @@ module Msf::Payload::Android
   def generate_jar(opts={})
     config = generate_config(opts)
     if opts[:stageless]
-      classes = MetasploitPayloads.read('android', 'meterpreter.dex')
+      classes = MetasploitPayloads.read('android', 'meterpreter.dexter')
       # Add stageless classname at offset 8000
       config += "\x00" * (8000 - config.size)
       config += 'com.metasploit.meterpreter.AndroidMeterpreter'
  • Start msfconsole
  • Verify that Metasploit starts
  • type the log command
  • Verify there are errors in the log like this:
[09/07/2019 07:28:03] [e(0)] core: Unable to build android/meterpreter_reverse_https, android/meterpreter.dexter not found.
[09/07/2019 07:28:03] [e(0)] core: Unable to build android/meterpreter_reverse_tcp, android/meterpreter.dexter not found.
[09/07/2019 07:28:03] [e(0)] core: Unable to build android/meterpreter_reverse_http, android/meterpreter.dexter not found.
make payload generation failures at boot time non-fatal
Currently, if any payload fails to generate that has a dynamic size, it causes a Framework instance to throw an exception on start. This can happen for a number of reasons, and more often than not it is enviromental (files missing, Y2k38 bugs, etc.). Instead of failing entirely, catch the exception and log as an error, don't register the payload, but continue booting.

@busterb busterb requested a review from bcoles Sep 7, 2019

@@ -91,6 +91,8 @@ def recalculate
sizes[name] = p.cached_size || p.new.size
# Don't cache generic payload sizes.
rescue NoCompatiblePayloadError
rescue StandardError => e
elog("Unable to build #{name}, #{e}.")

This comment has been minimized.

Copy link
@sempervictus

sempervictus Sep 7, 2019

Contributor

Thank you, this will save developers some time and headache.
For your next magic trick, got anything up your sleeve for fixing the ability to reload a payload module @ runtime when its being edited? The Linux shellcode PR using /lib code saved a lot of time, work on the module code required restarting framework.

Also, for this PR, an utter nitpick: unable to build payload #{name} due to #{e}

This comment has been minimized.

Copy link
@bcoles

bcoles Sep 8, 2019

Contributor

Usually we would use something like this:

elog("Call stack:\n#{e.backtrace.join "\n"}")

or:

elog("#{e.class} #{e.message}\n#{e.backtrace * "\n"}")

@h00die h00die self-assigned this Sep 8, 2019

@h00die

This comment has been minimized.

Copy link
Contributor

commented Sep 8, 2019

I'm going to take this and ninja patch in @sempervictus words

h00die added a commit that referenced this pull request Sep 8, 2019

@h00die h00die merged commit a985da9 into rapid7:master Sep 8, 2019

3 checks passed

Metasploit Automation - Sanity Test Execution Successfully completed all tests.
Details
Metasploit Automation - Test Execution Successfully completed all tests.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@h00die

This comment has been minimized.

Copy link
Contributor

commented Sep 8, 2019

Release Notes

Adds a log entry when payloads can't be loaded due to missing files and other such nuances to prevent crashing.

@bcoles
bcoles approved these changes Sep 8, 2019
Copy link
Contributor

left a comment

I haven't tested this, but LGTM.

msjenkins-r7 added a commit that referenced this pull request Sep 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.