Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix .rex_getaddrinfo inconsistencies #67

Merged
merged 4 commits into from
Nov 22, 2024

Conversation

zeroSteiner
Copy link
Contributor

The .rex_getaddrinfo method behaves inconsistently with Addrinfo.getaddrinfo, which makes .getaddresses inconsistent depending on if a resolver is present or not. When no resolver is present, a numeric address (either decimal or hex, not binary or octal) is specified, it's converted to an IPv4 address:

[2] pry(#<Msf::Modules::Exploit__Multi__Ssh__Sshexec::MetasploitModule>)> ::Addrinfo.getaddrinfo('0', 0, ::Socket::AF_UNSPEC, ::Socket::SOCK_STREAM)
=> [#<Addrinfo: 0.0.0.0 TCP (0)>]

But when a resolver is present, .getaddresses will call .rex_getaddrinfo and it falls with a resolution error because the address is not a valid hostname:

[10] pry(#<Msf::Modules::Exploit__Multi__Ssh__Sshexec::MetasploitModule>)> Rex::Socket.rex_getaddrinfo('0')
SocketError: getaddrinfo: Name or service not known
from /home/smcintyre/.rvm/gems/ruby-3.1.5@metasploit-framework/gems/rex-socket-0.1.57/lib/rex/socket.rb:996:in `rex_resolve_hostname'
[11] pry(#<Msf::Modules::Exploit__Multi__Ssh__Sshexec::MetasploitModule>)>

This came up as an issue that prevents Metasploit's route command from working when invoked as route add 0 0 -1 to add a default route for all IPv4 addresses through the most recent session when the DNS feature is enabled. This also removes references to ::Net::DNS in favor of using the constants from Dnsruby::Types instead, which was already referenced but not defined as a requirement. Adding Dnsruby as a requirement was needed to make the tests work.

lib/rex/socket.rb Outdated Show resolved Hide resolved
@adfoster-r7
Copy link
Contributor

Rebasing on master should fix the hanging builds, fixed by #68

Remove ::Net::DNS because it can be replaced by Dnsruby instead since we
just need the constants. Use Dnsruby because it's a public gem whereas
::Net::DNS is currently a vendored hack used by Metasploit.

One DNS library to rule them all.
This makes the implementation more consistent with Addrinfo.getaddrinfo.
@zeroSteiner
Copy link
Contributor Author

I just noticed that Addrinfo.getaddrinfo also handles normal IP addresses. I should really add that here as well. Let me get to that today to fix that inconsistency too.

[3] pry(#<Msf::Framework>)> hostname='127.0.0.1'
=> "127.0.0.1"
[4] pry(#<Msf::Framework>)> ::Addrinfo.getaddrinfo(hostname, 0, ::Socket::AF_UNSPEC, ::Socket::SOCK_STREAM)
=> [#<Addrinfo: 127.0.0.1 TCP>]
[5] pry(#<Msf::Framework>)> ::Addrinfo.getaddrinfo('::1', 0, ::Socket::AF_UNSPEC, ::Socket::SOCK_STREAM)
=> [#<Addrinfo: ::1 TCP>]
[6] pry(#<Msf::Framework>)> 

@zeroSteiner
Copy link
Contributor Author

Alright this should be ready to go now. I added normal IP address support in the last commit along with tests for it.

@adfoster-r7 adfoster-r7 merged commit 51a6f43 into rapid7:master Nov 22, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants