Fix LHOST validation rejecting tunnel hostnames when DNS lookup fails by stzifkas · Pull Request #21552 · rapid7/metasploit-framework

stzifkas · 2026-06-08T08:29:52Z

Summary

OptAddress#valid? was catching all DNS lookup failures and immediately returning false
This broke msfvenom payload generation when LHOST is set to a tunnel service hostname (ngrok, pinggy, etc.) that doesn't resolve from the attacker's machine but is perfectly valid syntax
Fix: on DNS failure, fall back to Rex::Socket.is_name? for hostname syntax validation — syntactically valid hostnames are accepted, garbage is still rejected

Verification steps

Reproduce with the installed framework before the fix:

msfvenom -p android/meterpreter/reverse_tcp LHOST=qhuoq-106-219-171-165.run.pinggy-free.link LPORT=4444 -o /tmp/test.apk
# Error: One or more options failed to validate: LHOST.

After the fix, the same command generates the APK successfully.
IPs, invalid hostnames, and garbage strings still fail validation as expected.

Related issues

See #21539

smcintyre-r7 · 2026-06-08T14:48:11Z

I'm pretty sure this is a regression I added with the work I did in #20989. I understand that payloads need a way to connect to a hostname, but having OptAddress pass validation for a hostname instead of either an IPv4 or IPv6 address seems like the type of thing that would cause issues down the line. Now the context here is the work in 20989 was intended to adjust SRVHOST which is similar to LHOST in the sense that typically they're the only address/hostname the target is supposed to use when connecting back to Metasploit. Both can be overridden by a different option (ListenerBindAddress for SRVHOST and ReverseListenerBindAddress for LHOST) which is what Metasploit binds to. When set, these must be resolvable by the local host, e.g. localhost.localdomain -> 127.0.0.1 is expected to work.

I think what we need here is possibly a new option that permits the hostname resolution to be deferred. It shouldn't be OptAddress though because it is fundamentally not an address. ReverseListenerBindAddress and ListenerBindAddress should stay OptAddress and resolve the hostname locally, failing when it's unable to. LHOST and SRVHOST should be something else, that's more permissive and either an address or a hostname. When it's a hostname, Metasploit shouldn't resolve it unless it needs to because ReverseListenerBindAddress and ListenerBindAddress are unset. I should note that this would be the default case, not an edge case.

dwelch-r7 · 2026-06-08T15:09:03Z

@smcintyre-r7 does this change make sense as is as a first step and we can create an issue to address the root issue more in depth? or are there specific issues you're concerned this will introduce that need to be addressed first?

Just thinking since this addresses a real problem now we could link the issue in a comment by the change while we work on a more holistic fix

smcintyre-r7 · 2026-06-08T18:27:05Z

If that ticket explicitly called out reverting the changes contained herein as part of the work and we pulled it into the next sprint then sure. Ideally though, we'd just fix it correctly the first time so it's up to @stzifkas on if they're up for the additional work that's necessary.

stzifkas · 2026-06-08T20:21:22Z

@smcintyre-r7 , @dwelch-r7 thanks for the feedback. Went ahead with the architectural approach you outlined. Here's what changed:

added OptAddressOrHostname as a new option type for LHOST. Validates via Rex::Socket.is_name? for hostname syntax and Rex::Socket.is_ip_addr? for IPs/interfaces, no DNS lookup at all. ReverseListenerBindAddress / ListenerBindAddress stay on OptAddress unchanged.
reverse.rb and reverse_udp.rb now skip resolv_nbo(LHOST) entirely when ReverseListenerBindAddress is explicitly set, since the bind address is already fully determined at that point.
tab_complete_source_interface changed to respond_to?(:interfaces) so interface name completion still works for LHOST.
SRVHOST is a separate story, its semantics vary per module (some want a local bind address, others a callback address) so is intentionally left out from this PR's scope.

smcintyre-r7 · 2026-06-08T20:21:31Z

+          # resolving LHOST (LHOST may be a tunnel hostname not locally resolvable).
+          bind_addr = datastore['ReverseListenerBindAddress']
+          any = Rex::Socket.is_ipv6?(bind_addr) ? "::0" : "0.0.0.0"
+          return [ (bind_addr == "0.0.0.0" || bind_addr == "::0") ? any : bind_addr ]


These checks for the any address should always normalize with addr_atoi then compare to zero. I know it's a preexisting issue but we shouldn't propagate it. The reason is actually apparent here, :: is typically the bind-any IPv6 address not ::0, but they're equivalent since per the RFC, the 0s can be omitted.

This comparison should always be:
Rex::Socket.addr_atoi(bind_addr) == 0 not bind_addr == "0.0.0.0" || bind_addr == "::0"). This is not the only location that needs to be updated with the correct pattern.

Done, using Rex::Socket.addr_atoi(bind_addr) == 0 throughout.

smcintyre-r7 · 2026-06-08T20:22:36Z

+    addr = Rex::Socket.resolv_nbo(datastore['LHOST'])
+    any = (addr.length == 4) ? "0.0.0.0" : "::0"


Use Rex::Socket.is_ipv4? not Rex::Socket.resolv_nbo(datastore['LHOST']).length == 4

Using Rex::Socket.is_ipv4? now.

smcintyre-r7 · 2026-06-08T20:24:01Z

+# syntactically valid hostname. Unlike OptAddress, DNS resolution is NOT
+# performed during validation, so tunnel service hostnames (ngrok, pinggy,
+# etc.) that are not resolvable from the local machine are accepted.


This would do well to allow an option like resolve_names: false and allow the caller to specify whether or not we should be resolving hostnames to IP addresses as part of the validation process. If we're performing a connection to the value or binding to it, that would allow us to fail earlier which would be extremely helpful.

resolve_names: is already there, defaults to false.

stzifkas · 2026-06-08T21:03:17Z

Addressed in the latest push

resolve_names: added to OptAddressOrHostname, and all wildcard address comparisons now use Rex::Socket.addr_atoi == 0 / Rex::Socket.is_ipv4? consistently across both handlers.

sjanusz-r7 · 2026-06-11T13:45:16Z

    register_options(
      [
-        OptAddressLocal.new('LHOST', [true, 'The local listener hostname']),
+        OptAddressOrHostname.new('LHOST', [true, 'The local listener hostname']),


For R7 reviewers: Going forward, we might look into modifying the option name here.

sjanusz-r7 · 2026-06-11T13:47:24Z

+    # @return [OptAddressOrHostname]
    def self.LHOST(default=nil, required=true, desc="The listen address (an interface may be specified)")
-      Msf::OptAddressLocal.new(__method__.to_s, [ required, desc, default ])
+      Msf::OptAddressOrHostname.new(__method__.to_s, [ required, desc, default ])


I think we don't want to change the default type for this option across all of MSF.

Reverted. Opt::LHOST is back to OptAddress. OptAddressOrHostname is only applied in the Reverse and ReverseUdp handlers.

sjanusz-r7 · 2026-06-11T13:50:13Z

+# a locally-resolvable address.
+#
+###
+class OptAddressOrHostname < OptBase


Can we deduplicate the validation and normalisation here and other options? Modifying the validation logic here to e.g. fix a bug, means other options don't get the same fix applied.

Now inherits from OptAddressRoutable. Duplicate interfaces, normalize_interface, and normalize_ip methods removed.

sjanusz-r7 · 2026-06-11T13:53:43Z

+  private
+
+  def normalize_interface(value)
+    addrs = NetworkInterface.addresses(value).values.flatten


This looks to be copy-pasted from opt_address_routable. Can we do any code deduplication here?

Removed, inherited from OptAddressRoutable now.

sjanusz-r7 · 2026-06-11T13:55:51Z

      end
    end
  end
+


Looks like we can restore this change

sjanusz-r7 · 2026-06-11T13:58:11Z

+    { value: 5 },
+    { value: [] },
+    { value: [1, 2] },
+    { value: {} },


Can we test nil as well?

Added nil to the invalid values list.

Removed nil from invalid_values - OptBase defaults required to false when attrs is empty, so nil is valid for that subject. The shared example already covers nil for both required and non-required cases.

sjanusz-r7 · 2026-06-11T13:59:45Z

        # XXX: We repurpose OptAddressLocal#interfaces, so we can't put this in Rex
        def tab_complete_source_interface(o)
-          return [] unless o.is_a?(Msf::OptAddressLocal)
+          return [] unless o.respond_to?(:interfaces)


We need to confirm if this change makes sense; or should we only tab complete when the option is an OptAddressLocal explicitly

Changed to is_a?(Msf::OptAddressRoutable) so it covers both OptAddressLocal and OptAddressOrHostname explicitly.

sjanusz-r7 · 2026-06-11T14:02:10Z

+    context 'with tunnel hostnames' do
+      it 'accepts without requiring DNS resolution' do
+        allow(::Rex::Socket).to receive(:getaddress).and_raise(::SocketError)
+        expect(opt.valid?('qhuoq-106-219-171-165.run.pinggy-free.link')).to be_truthy


Can we use any placeholder addresses here such as example.com as opposed to what looks like a real live link?

Replaced with tunnel.example.com throughout the spec.

sjanusz-r7 · 2026-06-11T14:04:37Z

+# a locally-resolvable address.
+#
+###
+class OptAddressOrHostname < OptBase


Could we either:
Inherit from OptAddress
or
Have the OptAddress as a member variable
in an effort to deduplicate?

Went with inheriting from OptAddressRoutable (which itself extends OptAddress). Deduplicates interfaces, normalize_interface, and normalize_ip_address.

stzifkas · 2026-06-12T10:05:04Z

Reverted Opt::LHOST back to OptAddress globally. OptAddressOrHostname is now only applied in Reverse and ReverseUdp handlers, which is where tunnel hostname support is actually needed. Rest of MSF is unaffected.

dwelch-r7 · 2026-06-23T12:53:54Z

@stzifkas I'm gonna be looking at this PR shortly but just to let you know you've got some failing tests that will need to be addressed

…on in bind_addresses

…idation

…e address family

stzifkas · 2026-06-23T16:40:24Z

Rebased on master. The python 3.8 macos build failure is pre-existing and unrelated to this PR.

Copilot

Pull request overview

This PR introduces a new option type to allow LHOST values that are syntactically valid hostnames even when they don’t resolve locally (common with tunnel providers), and updates reverse handlers to better separate “callback host” (LHOST) from “bind address” (ReverseListenerBindAddress).

Changes:

Added Msf::OptAddressOrHostname (accepts IP/interface/hostname without DNS by default, with an opt-in DNS resolution mode).
Updated reverse handlers to use the new option and to avoid resolving LHOST when ReverseListenerBindAddress is explicitly set.
Added RSpec coverage for the new option behavior (including tunnel-style hostnames) and updated interface tab completion to work with the new option’s superclass.

Impact Analysis:

Blast radius: medium; affects option validation for reverse handlers (and potentially any code using Opt::LHOST), impacting payload generation (msfvenom) and listener startup paths.
Data and contract effects: option validation semantics change for LHOST (hostname acceptance and when DNS is consulted); handler binding behavior changes when ReverseListenerBindAddress is set.
Rollback and test focus: validate msfvenom generation for staged and single reverse payloads, and validate listener bind behavior for IPv4/IPv6 + interface-name LHOST with/without ReverseListenerBindAddress.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
spec/lib/msf/core/opt_address_or_hostname_spec.rb	Adds specs covering tunnel hostname validation and normalization behaviors.
lib/msf/ui/console/module_option_tab_completion.rb	Broadens interface-name tab completion to `OptAddressRoutable`-derived options.
lib/msf/core/option_container.rb	Autoloads the new `OptAddressOrHostname` option type.
lib/msf/core/opt.rb	Changes the framework `Opt::LHOST` shortcut implementation.
lib/msf/core/opt_address_or_hostname.rb	Introduces the new option class implementing hostname-without-DNS validation.
lib/msf/core/handler/reverse.rb	Uses the new option for `LHOST` and adjusts bind address selection logic.
lib/msf/core/handler/reverse_udp.rb	Uses the new option for `LHOST` and adjusts bind address selection logic.
lib/msf/core/handler/reverse_http.rb	Switches `LHOST` option type to accept non-resolving hostnames.

stzifkas · 2026-06-24T15:16:22Z

+    # @return [OptAddress]
    def self.LHOST(default=nil, required=true, desc="The listen address (an interface may be specified)")
-      Msf::OptAddressLocal.new(__method__.to_s, [ required, desc, default ])
+      Msf::OptAddress.new(__method__.to_s, [ required, desc, default ])
    end


Fixed - now calls for IP addresses, so they go through OptAddressRoutable's multicast/reserved/broadcast checks. Wildcard IPs (0.0.0.0, 0::0) are now invalid for this option.

dwelch-r7 · 2026-06-24T14:50:38Z

+      if value =~ /^\d+(\.\d+)+$/
+        return Rex::Socket.is_ipv4?(value)
+      end
+
+      return true if Rex::Socket.is_ip_addr?(value)
+
+      if Rex::Socket.is_name?(value)
+        return true unless @resolve_names
+        begin
+          ::Rex::Socket.getaddress(value, true)
+          return true
+        rescue
+          return false
+        end
+      end


I think you just need to call super to address this, copilot is a bit wordy, but will need to verify that's all that's needed

Done. IP addresses now call super so they go through OptAddressRoutable for the multicast/reserved/broadcast checks. 0.0.0.0 and 0::0 are now invalid as a side effect (removed from the spec valid_values, added to invalid_values).

dwelch-r7 · 2026-07-06T11:00:40Z

+# -*- coding: binary -*-
+require 'network_interface'
+


Please address this

dwelch-r7 · 2026-06-24T12:48:30Z

-    # @return [OptAddressLocal]
+    # @return [OptAddress]
    def self.LHOST(default=nil, required=true, desc="The listen address (an interface may be specified)")
-      Msf::OptAddressLocal.new(__method__.to_s, [ required, desc, default ])
+      Msf::OptAddress.new(__method__.to_s, [ required, desc, default ])


I think we want to revert this change, if LHOST needs to be something else it should be up to the individual modules to override that like like you're doing elsewhere in the PR

Already done - Opt::LHOST is back to OptAddress. OptAddressOrHostname is only on the reverse handlers.

This has still changed? I think it needs to be reverted back to OptAddressLocal

dwelch-r7 · 2026-06-24T14:50:38Z

+      if value =~ /^\d+(\.\d+)+$/
+        return Rex::Socket.is_ipv4?(value)
+      end
+
+      return true if Rex::Socket.is_ip_addr?(value)
+
+      if Rex::Socket.is_name?(value)
+        return true unless @resolve_names
+        begin
+          ::Rex::Socket.getaddress(value, true)
+          return true
+        rescue
+          return false
+        end
+      end


I think you just need to call super to address this, copilot is a bit wordy, but will need to verify that's all that's needed

dwelch-r7 · 2026-06-24T14:51:36Z

+
+        # No ReverseListenerBindAddress set — resolve LHOST to determine the
+        # bind address and whether to use IPv4 or IPv6 ANY as fallback.
+        addr_nbo = Rex::Socket.resolv_nbo(datastore['LHOST'])


Won't this hit the same issue if LHOST is a domain name?

Good catch. Added a rescue around the resolv_nbo call in both reverse.rb and reverse_udp.rb - if LHOST is a hostname that cannot be resolved locally, it falls back to binding on 0.0.0.0 and prints a warning suggesting ReverseListenerBindAddress.

dwelch-r7 · 2026-06-24T14:56:46Z

I think we're missing tests for when resolve_names is true

Added - two tests: one where getaddress succeeds (hostname accepted) and one where it raises SocketError (hostname rejected).

dwelch-r7 · 2026-06-24T14:59:40Z

-    # Switch to IPv6 ANY address if the LHOST is also IPv6
-    addr = Rex::Socket.resolv_nbo(datastore['LHOST'])
-    # First attempt to bind LHOST. If that fails, the user probably has
-    # something else listening on that interface. Try again with ANY_ADDR.
-    any = (addr.length == 4) ? "0.0.0.0" : "::0"
-
-    addrs = [ Rex::Socket.addr_ntoa(addr), any  ]
-
    if not datastore['ReverseListenerBindAddress'].to_s.empty?
-      # Only try to bind to this specific interface
-      addrs = [ datastore['ReverseListenerBindAddress'] ]
-
-      # Pick the right "any" address if either wildcard is used
-      addrs[0] = any if (addrs[0] == "0.0.0.0" or addrs == "::0")
+      bind_addr = datastore['ReverseListenerBindAddress']
+      any = Rex::Socket.is_ipv6?(bind_addr) ? "::0" : "0.0.0.0"
+      return [ Rex::Socket.addr_atoi(bind_addr) == 0 ? any : bind_addr ]
    end

-    addrs
+    addr_nbo = Rex::Socket.resolv_nbo(datastore['LHOST'])
+    addr = Rex::Socket.addr_ntoa(addr_nbo)
+    any = Rex::Socket.is_ipv4?(addr) ? "0.0.0.0" : "::0"


This is missing some of the functionality of the bind_addresses method, namely missing is_loopback_address?

Added - bind_address now warns if LHOST resolves to a loopback address. ReverseUdp does not include the Reverse module so is_loopback_address? is not available there; inlined the equivalent check.

… resolution in bind_addresses

dwelch-r7 · 2026-06-25T12:59:03Z

+      # Reject anything that looks like a dotted-decimal number sequence
+      # (e.g. "192.0.2", "192.0.2.0.0") - Rex::Socket.is_name? accepts numeric
+      # DNS labels, so these would pass without this guard. Require it to be a
+      # valid IPv4 address, then delegate to parent for routable checks.
+      if value =~ /^\d+(\.\d+)+$/
+        return false unless Rex::Socket.is_ipv4?(value)
+        return super
+      end
+
+      return super if Rex::Socket.is_ip_addr?(value)


Suggested change

# Reject anything that looks like a dotted-decimal number sequence

# (e.g. "192.0.2", "192.0.2.0.0") - Rex::Socket.is_name? accepts numeric

# DNS labels, so these would pass without this guard. Require it to be a

# valid IPv4 address, then delegate to parent for routable checks.

if value =~ /^\d+(\.\d+)+$/

return false unless Rex::Socket.is_ipv4?(value)

return super

end

return super if Rex::Socket.is_ip_addr?(value)

return super if Rex::Socket.is_ip_addr?(value)

I think most of this is redundant

Done, removed the regex block. Also dropped 192.0.2 / 192.0.2.0.0 / 1.2.3.4.5 from invalid_values in the spec since those now pass as hostnames.

dwelch-r7 · 2026-07-01T13:21:20Z

@stzifkas seems like we have another test failure here, I think we're close to getting this landed though once that's passing I can give it one more manual test and we should be good to go

smcintyre-r7 added this to Metasploit Kanban Jun 8, 2026

github-project-automation Bot moved this to Todo in Metasploit Kanban Jun 8, 2026

smcintyre-r7 added library rn-fix release notes fix labels Jun 8, 2026

stzifkas force-pushed the fix/lhost-hostname-validation-dns-fallback branch from 1fe205f to 12708c2 Compare June 8, 2026 20:14

smcintyre-r7 requested changes Jun 8, 2026

View reviewed changes

github-project-automation Bot moved this from Todo to Waiting on Contributor in Metasploit Kanban Jun 8, 2026

stzifkas requested a review from smcintyre-r7 June 10, 2026 12:38

sjanusz-r7 reviewed Jun 11, 2026

View reviewed changes

bwatters-r7 moved this from Waiting on Contributor to In Progress in Metasploit Kanban Jun 15, 2026

bwatters-r7 moved this from In Progress to Ready in Metasploit Kanban Jun 15, 2026

dwelch-r7 self-assigned this Jun 22, 2026

stzifkas added 8 commits June 23, 2026 19:38

Fix LHOST validation to accept tunnel hostnames and skip DNS resoluti…

87e3fc8

…on in bind_addresses

Add resolve_names option to OptAddressOrHostname for optional DNS val…

5a13691

…idation

Use Rex::Socket.is_ipv4? instead of binary address length to determin…

8ce536e

…e address family

fix non-ascii chars in comments

6639b1d

fix LHOST type assertions in opt_spec

69284d7

address review feedback

9177163

scope OptAddressOrHostname to reverse handlers only

af17be2

remove nil from invalid_values

8bb40b3

stzifkas force-pushed the fix/lhost-hostname-validation-dns-fallback branch from 01061b6 to 8bb40b3 Compare June 23, 2026 16:39

dwelch-r7 requested a review from Copilot June 24, 2026 12:53

Copilot started reviewing on behalf of dwelch-r7 June 24, 2026 12:53 View session

Copilot AI reviewed Jun 24, 2026

View reviewed changes

dwelch-r7 reviewed Jun 24, 2026

View reviewed changes

dwelch-r7 mentioned this pull request Jun 24, 2026

Consolidate bind_address/bind_Addresses for reverse handlers #21601

Open

stzifkas added 3 commits June 24, 2026 18:15

delegate IP validation to parent for routable checks; rescue hostname…

bb1479f

… resolution in bind_addresses

add resolve_names: true tests

efd6f37

add loopback warning to bind_address in reverse_udp

6060b8f

dwelch-r7 reviewed Jun 25, 2026

View reviewed changes

dwelch-r7 mentioned this pull request Jun 26, 2026

Implement immediate payload option validation on set calls #21609

Open

15 tasks

dwelch-r7 moved this from Ready to In Progress in Metasploit Kanban Jun 29, 2026

stzifkas added 3 commits July 2, 2026 13:48

simplify valid? by removing redundant dotted-decimal guard

4208dd0

fix elog expectation: called twice when super traverses interfaces again

3b55bd7

remove redundant interfaces check - parent handles it through super

6073deb

		addr = Rex::Socket.resolv_nbo(datastore['LHOST'])
		any = (addr.length == 4) ? "0.0.0.0" : "::0"

Uh oh!

Conversation

stzifkas commented Jun 8, 2026

Summary

Verification steps

Related issues

Uh oh!

smcintyre-r7 commented Jun 8, 2026

Uh oh!

dwelch-r7 commented Jun 8, 2026

Uh oh!

smcintyre-r7 commented Jun 8, 2026

Uh oh!

stzifkas commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stzifkas commented Jun 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stzifkas commented Jun 12, 2026

Uh oh!

dwelch-r7 commented Jun 23, 2026

Uh oh!

stzifkas commented Jun 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

stzifkas commented Jun 8, 2026 •

edited

Loading

dwelch-r7 Jul 6, 2026 •

edited

Loading