Common Metasploit Module Coding Mistakes
Pages 99
- Home
- Adding Release Notes to PRs
- Committer Keys
- Committer Rights
- Common Metasploit Module Coding Mistakes
- Contributing to Metasploit
- Creating Metasploit Framework LoginScanners
- Debugging Dead Meterpreter Sessions
- Decommissioning Redmine
- Downloads by Version
- Evading Anti Virus
- Exploit Ranking
- Generating Module Documentation
- Git cheatsheet
- Git Gotchas
- Git Reference Sites
- Guidelines for Accepting Modules and Enhancements
- How payloads work
- How to add and update gems in metasploit framework
- How to check Microsoft patch levels for your exploit
- How to clean up files using FileDropper
- How to deprecate a Metasploit module
- How to do reporting or store data in module development
- How to get Oracle Support working with Kali Linux
- How to get started with writing a Meterpreter script
- How to get started with writing a post module
- How to get started with writing an auxiliary module
- How to get started with writing an exploit
- How to log in Metasploit
- How to obfuscate JavaScript in Metasploit
- How to parse an HTTP response
- How to Send an HTTP Request Using HTTPClient
- How to send an HTTP request using Rex::Proto::Http::Client
- How to use a Metasploit module appropriately
- How to use a reverse shell in Metasploit
- How to use command stagers
- How to use datastore options
- How to use exim_gethostbyname_bof.rb (Exim GHOST Buffer Overflow)
- How to use Msf::Auxiliary::AuthBrute to write a bruteforcer
- How to use msfvenom
- How to use PhpEXE to exploit an arbitrary file upload bug
- How to use Powershell in an exploit
- How to use Railgun for Windows post exploitation
- How to Use the FILEFORMAT mixin to create a file format exploit
- How to use the Msf::Exploit::Remote::Tcp mixin
- How to use the Seh mixin to exploit an exception handler
- How to use WbemExec for a write privilege attack on Windows
- How to write a browser exploit using BrowserExploitServer
- How to write a browser exploit using HttpServer
- How to write a check() method
- How to write a HTTP LoginScanner Module
- How to write a module using HttpServer and HttpClient
- How to zip files with Rex::Zip::Archive
- Indentation Standards
- Information About Unmet Browser Exploit Requirements
- Issue Labels
- Keeping in sync with rapid7 master
- Landing Meterpreter Pull Requests
- Landing Pull Requests
- Loading External Modules
- Metasploit development environment
- Metasploit Hackathons
- Metasploit Loginpalooza
- Metasploit module reference identifiers
- Meterpreter
- Meterpreter Configuration
- Meterpreter HTTP Communication
- Meterpreter Paranoid Mode
- Meterpreter Reliable Network Communication
- Meterpreter Sleep Control
- Meterpreter Stageless Mode
- Meterpreter Timeout Control
- Meterpreter Transport Control
- Meterpreter Unicode Support
- Meterpreter Wishlist
- Msftidy
- Nightly Installers
- Oracle Usage
- Payload Rename Justification
- Payload UUID
- Powershell Extension
- Python Extension
- Remote Branch Pruning
- Reporting a Bug
- Resuscitating Dead Pull Requests
- Rex Layout
- Rolling back merges
- Setting Up a Metasploit Development Environment
- Setting Up a Metasploit Development Environment Ubuntu 14.04
- State of Meterpreter
- Style Tips
- The ins and outs of HTTP and HTTPS communications in Meterpreter and Metasploit Stagers
- Uberhandler
- Unstable Modules
- Using Git
- Using Metasploit
- What does my Rex::Proto::SMB Error mean?
- Why Ruby?
- Writing Module Documentation
- Show 84 more pages…
Metasploit Wiki Pages
- Home Welcome to Metasploit!
- Using Metasploit A collection of useful links for penetration testers.
-
Setting Up a Metasploit Development Environment From
apt-get installtogit push. - CONTIBUTING.md What should your contributions look like?
- Landing Pull Requests Working with other people's contributions.
- Using Git All about Git and GitHub.
- Contributing to Metasploit Be a part of our open source community.
- Meterpreter All about the Meterpreter payload.
Clone this wiki locally
Deprecation notice!
Please see CONTRIBUTING.md for an authoritative coding guide. This document has fallen out of date. We don't write bad code any more! Hooray!
This is a collection of all the bad code we often see in Metasploit modules. You should avoid them, too.
Note: Some of these examples use puts() for demo purposes, but you should always use print_status / print_error when writing a module.
Bad Examples You Should NOT Follow:
- Not checking the return value of a Metasploit API
- Ruby 1.9.3 vs 1.8.7... gotcha!
- Not checking the return value when using match()
- Not checking nil before accessing a method
- Using exception handling to shut an error up
- Not taking advantage of the 'ensure' block
- Adding the 'VERBOSE' option
- Avoid using 'vars_post' for send_request_cgi() when crafting a POST request
- Bad variable naming style
- Using global variables
- Modifying the datastore during execution
1. Not checking the return value of a Metasploit API
res = send_request_cgi({
'method' => 'GET',
'uri' => '/app/index.php'
})
# There's a bug here, because res can return nil (due to a timeout or other reasons)
# If that happens, you will hit a "undefined method `code' for nil:NilClass" error.
# The correct way should be: if res && res.code == 200
if res.code == 200
print_status("Response looks good")
else
print_error("Unexpected response")
end2. Ruby 1.9.3 vs 1.8.7... gotcha!
some_string = "ABC"
# This can cause unexpected results to your module.
# Better to always do: char = some_string[1, 1]
char = some_string[1]
if char == 'B'
puts "You will see this message in Ruby 1.9.3"
elsif char == 66
puts "You will see this message in Ruby 1.8.7"
end# 1.9 allows a comma after the last argument when calling
# a method while 1.8 does not. The most common place to
# see this error is in the update_info() section in a
# module's constructor.
some_method(
"arg1",
"arg2", # <-- This comma is a syntax error on 1.8.x
)3. Not checking the return value when using match()
str = "dragon! drag on! Not lizard, I don't do that tongue thing"
# This tries to print "Not snake", but it's not in the string,
# so you'll get this error: "undefined method `[]' for nil:NilClass"
puts str.match(/(Not snake)/)[0]# The above is better written as:
if (str =~ /(Not snake)/)
puts $1
end4. Not checking nil first before accessing a method
str = "These things are round and tasty, let's call them... tastycles!"
food = str.scan(/donut holes/)[0]
# food is nil, and nil has no method called "empty".
# This will throw an error: "undefined method `empty?' for nil:NilClass"
if food.empty? or food.nil?
puts "I don't know what it's called"
end5. Using exception handling to shut an error up
begin
# This block has 2 issues:
# Issue #1: sample() is not a method in 1.8.7
# Issue #2: Divided by 0 (race condition)
n = [0, 1, 2, 3, 4, 5].sample
1/n
rescue
# If the user reports a bug saying this code isn't
# working, it can be hard to debug exactly what went
# wrong for the user without a backtrace.
# When you do this, the error also won't be logged in
# framework.log, either.
# Note that rescuing ::Exception is especially harmful
# because it can even hide syntax errors.
end6. Not taking advantage of the 'ensure' block
# You should use the ensure block to make sure x always has a value,
# which also avoids repeating code
begin
n = [0, 1, 2].sample
x = 1/n
rescue ZeroDivisionError => e
puts "Are you smarter than a 5th grader? #{e.message}"
x = 0 # Can put this in the ensure block
rescue NoMethodError
puts "You must be using an older Ruby"
x = 0 # Can put this in the ensure block
end
puts "Value is #{x.to_s}"7. Adding the 'VERBOSE' option
register_options(
[
# You already have this. Just type 'show advanced' and you'll see it.
# So no need to register again
OptBool.new("VERBOSE", [false, 'Enable detailed status messages', false])
], self.class)8. Avoid using send_request_cgi()'s vars_get or vars_get when crafting a POST/GET request
data_post = 'user=jsmith&pass=hello123'
# You should use the 'vars_post' key instead of 'data',
# unless you're trying to avoid the API escaping your
# parameter names
send_request_cgi({
'method' => 'POST',
'uri' => '/',
'data' => data_post
})9. Bad variable naming style
# What's this, Java?
# The proper naming style in this case should be: my_string
myString = "hello, world"10. Using global variables
# $msg is a global variable that can be accessed anywhere within the program.
# This can induce bugs to other modules or mixins that are hard to debug.
# Use @instance variables instead.
# This is also mentioned in your HACKING file :-)
class Opinion
def initialize
# This variable shouldn't be shared with other classes
$msg = "It's called the Freedom of Information Act. The Hippies finally got something right."
end
end
class Metasploit3
def initialize
puts $msg
end
end
Opinion.new
Metasploit3.new11. Modifying the datastore during execution
# https://github.com/rapid7/metasploit-framework/issues/3853
datastore['BAD'] = 'This is bad.'
