Skip to content

Commit

Permalink
Validate hostnames before storing them
Browse files Browse the repository at this point in the history
This proactively fixes a potential DoS condition where if a user were to
add a hostname containing a ; and followed by data that is not an IP
address that MSF may fail to start.

Example:
dns add-static 'foo;bar' 192.0.2.1
save
  • Loading branch information
zeroSteiner committed Feb 9, 2024
1 parent 99b2bfe commit 5036d28
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 6 deletions.
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,5 @@ group :test do
gem 'timecop'
end

# remove after https://github.com/rapid7/rex-socket/pull/65 is landed
gem 'rex-socket', git: 'https://github.com/zeroSteiner/rex-socket', branch: 'feat/util-is-name'
11 changes: 9 additions & 2 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
GIT
remote: https://github.com/zeroSteiner/rex-socket
revision: 764718bf3dd397c0a5ecc41ee0874d826e9c9144
branch: feat/util-is-name
specs:
rex-socket (0.1.56)
rex-core

PATH
remote: .
specs:
Expand Down Expand Up @@ -419,8 +427,6 @@ GEM
metasm
rex-core
rex-text
rex-socket (0.1.55)
rex-core
rex-sslscan (0.1.10)
rex-core
rex-socket
Expand Down Expand Up @@ -562,6 +568,7 @@ DEPENDENCIES
pry-byebug
rake
redcarpet
rex-socket!
rspec-rails
rspec-rerun
rubocop
Expand Down
14 changes: 11 additions & 3 deletions lib/msf/ui/console/command_dispatcher/dns.rb
Original file line number Diff line number Diff line change
Expand Up @@ -338,11 +338,16 @@ def add_static_dns(*args)
end

hostname = args.shift
if (ip_address = args.find { |a| !Rex::Socket.is_ip_addr?(a) })
if !Rex::Proto::DNS::StaticHostnames.is_valid_hostname?(hostname)
raise ::ArgumentError.new("Invalid hostname: #{hostname}")
end

ip_addresses = args
if (ip_address = ip_addresses.find { |a| !Rex::Socket.is_ip_addr?(a) })
raise ::ArgumentError.new("Invalid IP address: #{ip_address}")
end

args.each do |ip_address|
ip_addresses.each do |ip_address|
resolver.static_hostnames.add(hostname, ip_address)
print_status("Added static hostname mapping #{hostname} to #{ip_address}")
end
Expand Down Expand Up @@ -482,8 +487,11 @@ def remove_static_dns(*args)
end

hostname = args.shift
ip_addresses = args
if !Rex::Proto::DNS::StaticHostnames.is_valid_hostname?(hostname)
raise ::ArgumentError.new("Invalid hostname: #{hostname}")
end

ip_addresses = args
if ip_addresses.empty?
ip_addresses = resolver.static_hostnames.get(hostname, Dnsruby::Types::A) + resolver.static_hostnames.get(hostname, Dnsruby::Types::AAAA)
if ip_addresses.empty?
Expand Down
18 changes: 17 additions & 1 deletion lib/rex/proto/dns/static_hostnames.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def get1(hostname, type = Dnsruby::Types::A)
# @rtype [Array<IPAddr>]
def get(hostname, type = Dnsruby::Types::A)
hostname = hostname.downcase
@hostnames.fetch(hostname, {}).fetch(type, [])
@hostnames.fetch(hostname, {}).fetch(type, []).dup
end

# Add an IP address for the specified hostname.
Expand All @@ -78,6 +78,12 @@ def get(hostname, type = Dnsruby::Types::A)
# @param [IPAddr, String] ip_address The IP address that is being defined for the hostname. If this value is a
# string, it will be converted to an IPAddr instance.
def add(hostname, ip_address)
unless self.class.is_valid_hostname?(hostname)
# it is important to validate the hostname because assumptions about what characters it may contain are made
# when saving and loading it from the configuration
raise ::ArgumentError.new("Invalid hostname: #{hostname}")
end

ip_address = IPAddr.new(ip_address) if ip_address.is_a?(String) && Rex::Socket.is_ip_addr?(ip_address)

hostname = hostname.downcase
Expand Down Expand Up @@ -129,6 +135,16 @@ def delete(hostname, ip_address)
def flush
@hostnames.clear
end

def self.is_valid_hostname?(name)
# check if it appears to be a fully qualified domain name, e.g. www.metasploit.com
return true if Rex::Socket.is_name?(name)

# check if it appears to at least be a valid hostname, e.g. localhost
return true if (name =~ /^([a-z0-9][a-z0-9\-]{0,61})?[a-z0-9]$/i) && (name =~ /\s/).nil?

false
end
end
end
end
Expand Down

0 comments on commit 5036d28

Please sign in to comment.