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 get_resource nil bug in HttpServer#get_uri #12014

Merged
merged 1 commit into from Jun 28, 2019

Conversation

Projects
None yet
1 participant
@wvu-r7
Copy link
Contributor

commented Jun 26, 2019

This fixes a long-standing bug in HttpServer that I neglected to address.

If the module dev hasn't started the server yet, such as with start_service, get_resource returns nil, which get_uri - when called by the dev - attempts to concatenate with the rest of the URI.

A dev will see this bug only if they call get_uri out of order. It has happened before.

[1] pry(#<Msf::Modules::Exploit__Multi__Http__Jenkins_metaprogramming::MetasploitModule>)> get_uri
TypeError: no implicit conversion of nil into String
from /rapid7/metasploit-framework/lib/msf/core/exploit/http/server.rb:460:in `+'
[2] pry(#<Msf::Modules::Exploit__Multi__Http__Jenkins_metaprogramming::MetasploitModule>)> show-source get_uri

From: /rapid7/metasploit-framework/lib/msf/core/exploit/http/server.rb @ line 435:
Owner: Msf::Exploit::Remote::HttpServer
Visibility: public
Number of lines: 29

def get_uri(cli=self.cli)
  ssl = !!(datastore["SSL"])
  proto = (ssl ? "https://" : "http://")
  if datastore['URIHOST']
    host = datastore['URIHOST']
  elsif (cli and cli.peerhost)
    host = Rex::Socket.source_address(cli.peerhost)
  else
    host = srvhost_addr
  end

  if Rex::Socket.is_ipv6?(host)
    host = "[#{host}]"
  end

  if datastore['URIPORT'] && datastore['URIPORT'] != 0
    port = ':' + datastore['URIPORT'].to_s
  elsif (ssl and datastore["SRVPORT"] == 443)
    port = ''
  elsif (!ssl and datastore["SRVPORT"] == 80)
    port = ''
  else
    port = ":" + datastore["SRVPORT"].to_s
  end

  uri = proto + host + port + get_resource

  uri
end
[3] pry(#<Msf::Modules::Exploit__Multi__Http__Jenkins_metaprogramming::MetasploitModule>)> get_resource
=> nil
[4] pry(#<Msf::Modules::Exploit__Multi__Http__Jenkins_metaprogramming::MetasploitModule>)>

Noticed again while working on #11952. :)

@wvu-r7

This comment has been minimized.

Copy link
Contributor Author

commented Jun 28, 2019

Since I got a +1 from @RootUp, and no else has raised concerns, I'm landing this. :)

@wvu-r7 wvu-r7 self-assigned this Jun 28, 2019

@wvu-r7 wvu-r7 merged commit 01b308f into rapid7:master Jun 28, 2019

3 checks passed

Metasploit Automation - Sanity Test Execution Successfully completed all tests.
Details
Metasploit Automation - Test Execution Successfully completed all tests.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

wvu-r7 added a commit that referenced this pull request Jun 28, 2019

msjenkins-r7 added a commit that referenced this pull request Jun 28, 2019

@wvu-r7

This comment has been minimized.

Copy link
Contributor Author

commented Jun 28, 2019

Release Notes

This fixes a crash in modules that call the get_uri method from the HttpServer library before the service has started.

@wvu-r7 wvu-r7 deleted the wvu-r7:bug/http branch Jun 28, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.