From 8f4f51874dfd99648b22de3b5fea99832da90b90 Mon Sep 17 00:00:00 2001 From: antisnatchor Date: Thu, 6 Mar 2014 17:11:27 +0000 Subject: [PATCH] Fixed issues with the DNS server RESTful API. Now it works. --- extensions/dns/dns.rb | 27 +++++++++++---------------- extensions/dns/rest/dns.rb | 23 ++++++++++++++--------- extensions/dns/ruby/rubydns.rb | 11 +++++++++-- 3 files changed, 34 insertions(+), 27 deletions(-) diff --git a/extensions/dns/dns.rb b/extensions/dns/dns.rb index 6ebd71571..6803de477 100644 --- a/extensions/dns/dns.rb +++ b/extensions/dns/dns.rb @@ -29,6 +29,14 @@ module BeEF @server = nil end + def set_server(server) + @server = server + end + + def get_server + @server + end + # Starts the main DNS server run-loop in a new thread. # # @param address [String] interface address server should run on @@ -36,21 +44,12 @@ module BeEF def run_server(address = '0.0.0.0', port = 5300) @address = address @port = port - - @lock.synchronize do Thread.new do - # @note Calling #sleep is a quick fix that prevents race conditions - # with WebSockets. A better solution is needed; perhaps a - # global EventMachine mutex. - sleep(1) + sleep(2) - if EventMachine.reactor_running? - EventMachine.next_tick { run_server_block(@address, @port) } - else - run_server_block(@address, @port) - end + # antisnatchor: RubyDNS is already implemented with EventMachine + run_server_block(@address, @port) end - end end # Adds a new DNS rule or "resource record". Does nothing if rule is already present. @@ -132,11 +131,7 @@ module BeEF # @param port [Integer] desired server port number def run_server_block(address, port) RubyDNS.run_server(:listen => [[:udp, address, port]]) do - server = self - BeEF::Extension::Dns::Server.instance.instance_eval { @server = server } - # Pass unmatched queries upstream to root nameservers - server = [] dns_config = BeEF::Core::Configuration.instance.get('beef.extension.dns') unless dns_config['upstream'].nil? dns_config['upstream'].each do |server| diff --git a/extensions/dns/rest/dns.rb b/extensions/dns/rest/dns.rb index e1ffbf826..4413c7ec7 100644 --- a/extensions/dns/rest/dns.rb +++ b/extensions/dns/rest/dns.rb @@ -77,10 +77,6 @@ module BeEF unless [pattern, type, response].include?(nil) # Determine whether 'pattern' is a String or Regexp begin - # antisnatchor: UNSAFE EVAL!!! RCE - #pattern_test = eval pattern - #pattern = pattern_test if pattern_test.class == Regexp - # if pattern is a Regexp, then create a new Regexp object if %r{\A/(.*)/([mix]*)\z} =~ pattern pattern = Regexp.new(pattern) @@ -96,6 +92,18 @@ module BeEF raise InvalidJsonError, 'Non-array "response" key passed to endpoint /api/dns/rule' end + safe_response = true + response.each do |ip| + unless BeEF::Filters.is_valid_ip?(ip) + safe_response = false + break + end + end + + unless safe_response + raise InvalidJsonError, 'Invalid IP in "response" key passed to endpoint /api/dns/rule' + end + unless BeEF::Filters.is_non_empty_string?(pattern) raise InvalidJsonError, 'Empty "pattern" key passed to endpoint /api/dns/rule' end @@ -105,16 +113,13 @@ module BeEF end id = '' - block_src = format_response(type, response) # antisnatchor: would be unsafe eval, but I added 2 validations before (alpha-num only and list of valid types) + # Now it's safe type_obj = eval "Resolv::DNS::Resource::IN::#{type}" - # Bypass #add_rule so that 'block_src' can be passed as a String - BeEF::Extension::Dns::Server.instance.instance_eval do - id = @server.match(pattern, type_obj, block_src) - end + id = BeEF::Extension::Dns::Server.instance.get_server.match(pattern, type_obj, block_src) result = {} result['success'] = true diff --git a/extensions/dns/ruby/rubydns.rb b/extensions/dns/ruby/rubydns.rb index 43214d55c..0a8d13d77 100644 --- a/extensions/dns/ruby/rubydns.rb +++ b/extensions/dns/ruby/rubydns.rb @@ -20,6 +20,8 @@ module RubyDNS def self.run_server(options = {}, &block) server = RubyDNS::Server.new(&block) + BeEF::Extension::Dns::Server.instance.set_server(server) + options[:listen] ||= [[:udp, '0.0.0.0', 53], [:tcp, '0.0.0.0', 53]] EventMachine.run do @@ -60,7 +62,8 @@ module RubyDNS BeEF::Core::Models::Dns::Rule.each do |rule| id = rule.id pattern = [rule.pattern, rule.type] - #TODO antisnatchor: potentially unsafe (although input is from data already stored in the databse) + # antisnatchor: this would be unsafe, but input gets validated in extensions/dns/rest/dns.rb (lines 95 to 105) + # in this case input comes from the DB, but that data stored in the DB was originally coming from the now safe code block = eval rule.block regex = pattern[0] @@ -97,9 +100,13 @@ module RubyDNS id = generate_id + if @rules == nil + @rules = [] + end + case block when String - #TODO antisnatchor: potentially unsafe (make sure block_src is safe or change this logic) + # antisnatchor: this would be unsafe, but input gets validated in extensions/dns/rest/dns.rb (lines 95 to 105) @rules << Rule.new(id, pattern, eval(block_src)) when Proc @rules << Rule.new(id, pattern, block)