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

Add support to msfvenom for "-f octal" #18363

Merged
merged 7 commits into from
Oct 3, 2023

Conversation

j0ev
Copy link
Contributor

@j0ev j0ev commented Sep 13, 2023

This PR adds an octal transform format to msfvenom for use during payload generation. This is useful for printf in POSIX sh.

Verification

List the steps needed to make sure this thing works

  • make sure 'raw' still works: ./msfvenom -p linux/aarch64/shell_reverse_tcp lhost=127.0.0.1 lport=5555 -f raw
  • check that the new 'octal' works: ./msfvenom -p linux/aarch64/shell_reverse_tcp lhost=127.0.0.1 lport=5555 -f octal
  • check that the new 'octal' works in msfconsole ./msfconsole use some_payload generate -f octal

@cgranleese-r7
Copy link
Contributor

cgranleese-r7 commented Sep 15, 2023

Currently this doesn't work for me when testing this step:

  • check that the new octal works in msfconsole ./msfconsole use some_payload generate -f octal

I get the following error:[-] Payload generation failed: Unsupported buffer format: octa. I did a quick pry and it hit this line:

raise BufferFormatError, "Unsupported buffer format: #{fmt}", caller

I believe this is due to rex-text needing to be updated as well. I took a quick look and it seems you'd need something similar to what was done here when Masm was added.

Then a bump will be needed for the rex-text gem in metasploit-framework and you will be able to rebase this PR to pull in those changes.

I suspect you'll also need to add another change to this PR, this is Masm PR for the metasploit-framework side of things. Specifically those two lines I linked are what I think will need added.

@j0ev
Copy link
Contributor Author

j0ev commented Sep 15, 2023

Thanks for catching that. I am not familiar with what the to_comment functions were intended for, I will take a look and open a rex PR if needed.

@j0ev
Copy link
Contributor Author

j0ev commented Sep 15, 2023

Added a test for this scenario, which should fail atm.

@adfoster-r7
Copy link
Contributor

Added a test for this scenario, which should fail atm.

Looks like all the tests went green 👀

@j0ev
Copy link
Contributor Author

j0ev commented Sep 20, 2023

Whoops, got confused on the code path. Added a new in-progress spec for the situation of "valid transform without a corresponding to_comment method".

* Adds failing test that discovers several additional violations.
@j0ev
Copy link
Contributor Author

j0ev commented Sep 20, 2023

@cgranleese-r7 after looking at this closely I think it's OK to just copy what is done for the hex format. hex and octal are a little different from some of the other formats (c, golang, etc) in that they're just a string encoding, not a language; thus they don't really have a "comment" format per-se. This would also avoid a Rex PR. Wdyt?

I wrote a spec for invoking the code path that gets called from msfconsole's generate subcommand. It turns up a number of other identical situations with other formats:

rspec './spec/lib/msf/simple/payload_spec.rb[1:33:1]' # Msf::Simple::Payload when given the transform format 'vbscript' is expected not to raise Exception
rspec './spec/lib/msf/simple/payload_spec.rb[1:3:1]' # Msf::Simple::Payload when given the transform format 'base64' is expected not to raise Exception
rspec './spec/lib/msf/simple/payload_spec.rb[1:32:1]' # Msf::Simple::Payload when given the transform format 'vbapplication' is expected not to raise Exception
rspec './spec/lib/msf/simple/payload_spec.rb[1:2:1]' # Msf::Simple::Payload when given the transform format 'base32' is expected not to raise Exception

I think base64 and base32 make sense to lump in with hex and octal. The other ones I'm not so sure.

@cgranleese-r7
Copy link
Contributor

I didn't spend a whole lot of time looking into it. Just when it wasn't working for me initially I took quick look at other format implementations.

@cgranleese-r7 after looking at this closely I think it's OK to just copy what is done for the hex format. hex and octal are a little different from some of the other formats (c, golang, etc) in that they're just a string encoding, not a language; thus they don't really have a "comment" format per-se. This would also avoid a Rex PR. Wdyt?

It does seem to be working for me now. So seems the rex-text PR may not be required.

make sense to lump in with hex and octal

Yea, I think having it in with hex makes sense as well.

* Explicitly documents lack of support for VB style comments.
@j0ev
Copy link
Contributor Author

j0ev commented Sep 21, 2023

Okay, I think this is ready for re-review. The vbscript and vbapplication transform formats still have no comment (reading on it, comments in VB start with '), but this is pre-existing (and now unit-tested) behavior and I would rather avoid a Rex PR here.

@bwatters-r7 bwatters-r7 self-assigned this Oct 2, 2023
@bwatters-r7 bwatters-r7 merged commit 6aeffa5 into rapid7:master Oct 3, 2023
32 checks passed
@bwatters-r7
Copy link
Contributor

Release Notes

This PR adds support to outputting payloads in octal in both framework and venom.

@bwatters-r7 bwatters-r7 added feature rn-enhancement release notes enhancement and removed feature labels Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rn-enhancement release notes enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants