Mixins defining `connect` can cause issues with other mixins

[adfoster-r7]

Problem

Various mixins define a connect method which can interfere with each other when multiple mixins are used. For instance with a nodule that requires both an http client and psexec mixins:

class MetasploitModule < Msf::Exploit::Remote
  include Msf::Exploit::Remote::HttpClient
  include Msf::Exploit::Remote::SMB::Client::Psexec
  def initialize(info={})
    ...
  end
  def exploit
    ...
    send_request_cgi(...)
    ...
    smb_login()
    ...
    end
end

When running this module, the following exception is thrown when the send_request_cgi method is invoked:

ArgumentError unknown keyword: :cgi

It turns out that both the http client mixin, and the smb module have a connect method. The last definition of connect wins. Therefore, depending on the order of mixin includes, when the http client is intending to call its own internal http connect method, it can end up calling the smb client connect method instead.

This problem has occurred in the past, and is most likely present for multiple mixins: https://github.com/rapid7/metasploit-framework/pull/13093

An elegant solution to solve this issue would be great. I have a feeling this is a duplicate issue, but I can't find an existing issue that mentions this particular problem.

[smcintyre-r7]

We don't see too many vulnerabilities that require interaction with the target over multiple protocols. They do exist, they're just exceedingly rare. Given the subset of vulnerabilities however that do require interaction with the target over multiple protocols, all the examples that I'm aware of involve HTTP as one of the two protocols. Because of that, I would propose addressing this issue for the vast majority of cases by implementing a new HTTP client API (which we've discussed wanting to do anyways) that takes an object-orientated approach and therefor doesn't need a connect method that could be affected by whatever the other protocol is.

[github-actions[bot]]

Hi!

This issue has been left open with no activity for a while now.

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 30 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!

As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request.