Fixed issues with the DNS server RESTful API. Now it works.

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|

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

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)