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 encoders module : msf > set ruby/base32 #10921

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@ismailtasdelen

ismailtasdelen commented Nov 5, 2018

Tell us what this change does. If you're fixing a bug, please mention
the github issue number.

Verification

List the steps needed to make sure this thing works

  • Start msfconsole
  • use exploit/windows/smb/ms17_010_eternalblue
  • show encoders
  • set ruby/base32
  • ...
  • Verify the thing does what it should
  • Verify the thing does not do what it should not
  • Document the thing and how it works (Example)

#10915

ismailtasdelen added some commits Nov 5, 2018

add encoders module : msf > set ruby/base32
add encoders module : msf > set ruby/base32
@bcoles

This comment has been minimized.

Contributor

bcoles commented Nov 5, 2018

master is not a unique branch. This needs to be committed from a unique branch, as per #10915 (comment).

Also, this clearly hasn't been tested.

Also, what does the base32 encoder offer over the base64 encoder?

Also, the comment header is malformed.

This:

 This module requires Metasploit: https://metasploit.com/download
# Current source: https://github.com/rapid7/metasploit-framework
##

Should be this:

##
# This module requires Metasploit: https://metasploit.com/download
# Current source: https://github.com/rapid7/metasploit-framework
##
@bwatters-r7

This comment has been minimized.

Contributor

bwatters-r7 commented Nov 5, 2018

Please also run msftidy on this, as it is failing because of:

modules/encoders/ruby/base32.rb - [INFO] No CVE references found. Please check before you land!
modules/encoders/ruby/base32.rb:1 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:2 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:3 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:4 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:5 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:6 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:7 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:8 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:9 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:10 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:11 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:12 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:13 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:14 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:15 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:16 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:17 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:18 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:19 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:20 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:21 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:22 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:23 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:24 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:25 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:26 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:27 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:28 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:29 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:30 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:31 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb:32 - [WARNING] Carriage return EOL
modules/encoders/ruby/base32.rb - [WARNING] Please add a newline at the end of the file

You can find msftidy in the tools/dev directory and run it manually before pushing your changes to verify they are correct.

If you are new to Ruby, please also check out Rubocop: https://github.com/rapid7/metasploit-framework/wiki/Using-Rubocop

@bwatters-r7

This comment has been minimized.

Contributor

bwatters-r7 commented Nov 5, 2018

Hi there @ismailtasdelen! Sorry if that was confusing. We have different people coming and helping us out, so sometimes we can be a bit quick in our answers. I'd be happy to help with this, so please feel free to ask- you can be sure that I get it by adding @bwatters-r7 in your reply.

I see you got some of the changes from @bcoles done, and that's great. His comment about master was that you have pushed up your master branch rather than a topic branch. The catch there is that you will likely keep using your master branch, so in the future, we will not be able to compare the code well. You can read more and see an example of why we don't want that to happen here: https://github.com/rapid7/metasploit-framework/blob/master/CONTRIBUTING.md

The EOL errors from msftidy are because the line endings in the file are in Windows style instead of Unix. That's easy to fix, but usually it depends on the editor or the OS you are using. In Linux, you could use dos2unix or fromdos to fix the line endings, but it is also likely an option if you are using a full-featured editor. If you need help, let me know what editor you are using and I can give you some better guidance. Alternatively, I could do it and push to this branch, but that would make it look like I wrote the file, since I would end up changing all those lines.

It would also help if you took a few lines to tell us why you want to add this. I am not totally an expert at encoders and why I might use one over another, so in this case, is there a situation you found that this to succeed when our current encoders fail? I'm also fine if this is just faster/better/more extensible. It just helps us to know why this helps out users.

Thanks!

@bcoles

This comment has been minimized.

Contributor

bcoles commented Nov 6, 2018

This is a Base64 decoder, not a Base32 decoder.

2.3.0 :001 > require 'rex'
/usr/local/rvm/gems/ruby-2.3.0/gems/librex-0.0.999/lib/rex/proto/http/server.rb:83: warning: key "jpeg" is duplicated and overwritten on line 84
/usr/local/rvm/gems/ruby-2.3.0/gems/librex-0.0.999/lib/rex/proto/smb/exceptions.rb:11: warning: key 0 is duplicated and overwritten on line 12
/usr/local/rvm/gems/ruby-2.3.0/gems/librex-0.0.999/lib/rex/proto/smb/exceptions.rb:17: warning: key 128 is duplicated and overwritten on line 18
 => true 
2.3.0 :002 > buf = 'test'
 => "test" 
2.3.0 :003 > b32 = Rex::Text.encode_base32(buf)
 => "ORSXG5A=" 
2.3.0 :004 > eval(%(" + b32 + ").unpack(%(m0)).first)
ArgumentError: invalid base64
	from (irb):4:in `unpack'
	from (irb):4
	from /usr/local/rvm/rubies/ruby-2.3.0/bin/irb:11:in `<main>'
2.3.0 :005 > 
@wvu-r7

This comment has been minimized.

Contributor

wvu-r7 commented Nov 6, 2018

Yep, m0 is Base64, specifically RFC 4648. Please try again. And from a topic branch, not your master branch. Thank you.

@wvu-r7 wvu-r7 closed this Nov 6, 2018

@ismailtasdelen ismailtasdelen referenced this pull request Nov 6, 2018

Closed

add module base32 encoder #10915

6 of 6 tasks complete
@jmartin-r7

This comment has been minimized.

Contributor

jmartin-r7 commented Nov 6, 2018

It is required that code in your fork be merged from a unique branch in your repository to master in Rapid7's. Please create a new branch in your fork of framework and resubmit this from that branch.

git checkout -b <BRANCH_NAME>
git push <your_fork_remote> <BRANCH_NAME>

This helps protect the process, ensure users are aware of commits on the branch being considered for merge, allows for a location for more commits to be offered without mingling with other contributor changes and allows contributors to make progress while a PR is still being reviewed.

Closing based on the this requirement, please do resubmit from a unique branch.

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