Clearer RHOST error message

lib/msf/core/option_container.rb

@@ -217,10 +217,15 @@ def validate(datastore)
         rhosts_walker = Msf::RhostsWalker.new(datastore['RHOSTS'], datastore)
         rhosts_count = rhosts_walker.count
         unless rhosts_walker.valid?
-          invalid_values = rhosts_walker.to_enum(:errors).take(5).map(&:value)
+          errors = rhosts_walker.to_enum(:errors).to_a
+          grouped = errors.group_by { |err| err.cause.nil? ? nil : (err.cause.class.const_defined?(:MESSAGE) ? err.cause.class::MESSAGE : nil) }
           error_options << 'RHOSTS'
-          if invalid_values.any?
-            error_reasons['RHOSTS'] << "unexpected values: #{invalid_values.join(', ')}"
+          if grouped.any?
+            grouped.each do | message, error_subset |
+              invalid_values = error_subset.map(&:value).take(5)
+              message = 'Unexpected values' if message.nil?
+              error_reasons['RHOSTS'] << "#{message}: #{invalid_values.join(', ')}"
+            end
           end
         end
 

lib/msf/core/rhosts_walker.rb

@@ -39,9 +39,15 @@ def initialize(value, msg = "Unexpected rhost value: #{value.inspect}", cause: n
     end
 
     class InvalidSchemaError < StandardError
+      MESSAGE = 'Invalid schema'
     end
 
     class InvalidCIDRError < StandardError
+      MESSAGE = 'Invalid CIDR'
+    end
+
+    class RhostResolveError < StandardError
+      MESSAGE = 'Host resolution failed'
     end
 
     def initialize(value = '', datastore = Msf::ModuleDataStore.new(nil))
@@ -135,20 +141,30 @@ def parse(value, datastore)
             schema = Regexp.last_match(:schema)
             raise InvalidSchemaError unless SUPPORTED_SCHEMAS.include?(schema)
 
+            found = false
             parse_method = "parse_#{schema}_uri"
             parsed_options = send(parse_method, value, datastore)
             Rex::Socket::RangeWalker.new(parsed_options['RHOSTS']).each_ip do |ip|
               results << datastore.merge(
                 parsed_options.merge('RHOSTS' => ip, 'UNPARSED_RHOSTS' => value)
               )
+              found = true
+            end
+            unless found
+              raise RhostResolveError.new(value)
             end
           else
+            found = false
             Rex::Socket::RangeWalker.new(value).each_host do |rhost|
               overrides = {}
               overrides['UNPARSED_RHOSTS'] = value
               overrides['RHOSTS'] = rhost[:address]
               set_hostname(datastore, overrides, rhost[:hostname])
               results << datastore.merge(overrides)
+              found = true
+            end
+            unless found
+              raise RhostResolveError.new(value)
             end
           end
         rescue ::Interrupt

spec/lib/msf/core/option_container_spec.rb

@@ -160,7 +160,7 @@
         expect { options_with_rhosts.validate(datastore) }.to raise_error(Msf::OptionValidateError) { |error|
           expected_reasons = {
             'RHOSTS' => [
-              'unexpected values: http://198.51.100.1:8080path, http://foo:bar@198.51.100.1:8080path'
+              'Unexpected values: http://198.51.100.1:8080path, http://foo:bar@198.51.100.1:8080path'
             ]
           }
           expect(error.options).to eq(['RHOSTS', 'HttpUsername', 'HttpPassword'])

spec/lib/msf/core/rhosts_walker_spec.rb

@@ -40,12 +40,16 @@ def http_options_for(datastores)
     return false if actual.count != expected.count
 
     expected.zip(actual).all? do |(expected, actual)|
-      actual.instance_of?(expected.class) && actual.message == expected.message
+
+      same_message = actual.instance_of?(expected.class) && actual.message == expected.message
+      same_cause = (actual.cause.nil? && expected.cause.nil?) || (actual.cause.class == expected.cause.class)
+
+      same_message && same_cause
     end
   end
 
   failure_message do |actual|
-    "\nexpected:\n#{expected.to_a.join(",\n")}\n\ngot:\n#{actual.to_a.join(",\n")}\n\n(compared using ==)\n"
+    "\nexpected:\n#{expected.to_a.map {|e| "#{e.message} (#{e.cause})"}.join(",\n")}\n\ngot:\n#{actual.to_a.map {|e| "#{e.message} (#{e.cause})"}.join(",\n")}\n\n(compared using ==)\n"
   end
 
   failure_message_when_negated do |_actual|
@@ -321,6 +325,9 @@ def each_error_for(mod)
   before(:each) do
     @temp_files = []
 
+    allow(::Addrinfo).to receive(:getaddrinfo).with('nonexistent.com', 0, ::Socket::AF_UNSPEC, ::Socket::SOCK_STREAM) do |*_args|
+      []
+    end
     allow(::Addrinfo).to receive(:getaddrinfo).with('example.com', 0, ::Socket::AF_UNSPEC, ::Socket::SOCK_STREAM) do |*_args|
       [::Addrinfo.new(['AF_INET', 0, 'example.com', '192.0.2.2'])]
     end
@@ -420,20 +427,23 @@ def create_tempfile(content)
       { 'expected' => [] },
       { 'RHOSTS' => nil, 'expected' => [] },
       { 'RHOSTS' => '', 'expected' => [] },
-      { 'RHOSTS' => '-1', 'expected' => [] },
-      { 'RHOSTS' => 'http:', 'expected' => [Msf::RhostsWalker::Error.new('http:')] },
-      { 'RHOSTS' => '127.0.0.1 http:', 'expected' => [Msf::RhostsWalker::Error.new('http:')] },
-      { 'RHOSTS' => '127.0.0.1 http: 127.0.0.1 https:', 'expected' => [Msf::RhostsWalker::Error.new('http:'), Msf::RhostsWalker::Error.new('https:')] },
+      { 'RHOSTS' => '-1', 'expected' => [Msf::RhostsWalker::Error.new('-1', cause: Msf::RhostsWalker::RhostResolveError.new)] },
+      { 'RHOSTS' => 'http:', 'expected' => [Msf::RhostsWalker::Error.new('http:', cause: Addressable::URI::InvalidURIError.new)] },
+      { 'RHOSTS' => '127.0.0.1 http:', 'expected' => [Msf::RhostsWalker::Error.new('http:', cause: Addressable::URI::InvalidURIError.new)] },
+      { 'RHOSTS' => '127.0.0.1 http: 127.0.0.1 https:', 'expected' => [Msf::RhostsWalker::Error.new('http:', cause: Addressable::URI::InvalidURIError.new), Msf::RhostsWalker::Error.new('https:', cause: Addressable::URI::InvalidURIError.new)] },
       # Unknown protocols aren't validated, as there may be potential ambiguity over ipv6 addresses
       # which may technically start with a 'schema': AAA:1450:4009:822::2004. The existing rex socket
       # range walker will silently drop this value though, and it may be treated as an overall error.
-      { 'RHOSTS' => 'unknown_protocol://127.0.0.1 127.0.0.1', 'expected' => [ ] },
+      { 'RHOSTS' => 'unknown_protocol://127.0.0.1 127.0.0.1', 'expected' => [ Msf::RhostsWalker::Error.new('unknown_protocol://127.0.0.1', cause: Msf::RhostsWalker::RhostResolveError.new)] },
 
       # cidr validation
-      { 'RHOSTS' => 'cidr:127.0.0.1', 'expected' => [Msf::RhostsWalker::Error.new('cidr:127.0.0.1')] },
-      { 'RHOSTS' => 'cidr:abc/127.0.0.1', 'expected' => [Msf::RhostsWalker::Error.new('cidr:abc/127.0.0.1')] },
-      { 'RHOSTS' => 'cidr:/1000:127.0.0.1', 'expected' => [Msf::RhostsWalker::Error.new('cidr:/1000:127.0.0.1')] },
-      { 'RHOSTS' => 'cidr:%eth2:127.0.0.1', 'expected' => [Msf::RhostsWalker::Error.new('cidr:%eth2:127.0.0.1')] },
+      { 'RHOSTS' => 'cidr:127.0.0.1', 'expected' => [Msf::RhostsWalker::Error.new('cidr:127.0.0.1', cause: Msf::RhostsWalker::InvalidCIDRError.new)] },
+      { 'RHOSTS' => 'cidr:abc/127.0.0.1', 'expected' => [Msf::RhostsWalker::Error.new('cidr:abc/127.0.0.1', cause: Msf::RhostsWalker::InvalidCIDRError.new)] },
+      { 'RHOSTS' => 'cidr:/1000:127.0.0.1', 'expected' => [Msf::RhostsWalker::Error.new('cidr:/1000:127.0.0.1', cause: Msf::RhostsWalker::InvalidCIDRError.new)] },
+      { 'RHOSTS' => 'cidr:%eth2:127.0.0.1', 'expected' => [Msf::RhostsWalker::Error.new('cidr:%eth2:127.0.0.1', cause: Msf::RhostsWalker::InvalidCIDRError.new)] },
+
+      # host resolution
+      { 'RHOSTS' => 'https://nonexistent.com:9000/foo', 'expected' => [Msf::RhostsWalker::Error.new('https://nonexistent.com:9000/foo', cause: Msf::RhostsWalker::RhostResolveError.new)] },
     ].each do |test|
       it "handles the input #{test['RHOSTS'].inspect} as having the errors #{test['expected']}" do
         aux_mod.datastore['RHOSTS'] = test['RHOSTS']