[Draft] Scanner module check method implementation is wrong

[adfoster-r7]

The current implementation of scanner doesn't walk RHOST ranges as expected:

https://github.com/rapid7/metasploit-framework/blob/3c0222d7d0cdde4ff8aebb74672624964988e038/lib/msf/core/auxiliary/scanner.rb#L38-L45

The expectation should be that the check functionality will iterate over the RHOST range in a similar fashion to the run command, which parses the RHOST string and creates its own threads etc:

https://github.com/rapid7/metasploit-framework/blob/3c0222d7d0cdde4ff8aebb74672624964988e038/lib/msf/core/auxiliary/scanner.rb#L60 https://github.com/rapid7/metasploit-framework/blob/3c0222d7d0cdde4ff8aebb74672624964988e038/lib/msf/core/auxiliary/scanner.rb#L100-L118

Note: This will require reviewing the existing modules that implement check methods, as their implementation is most likely wrong - and check_host should be implemented instead:

$ rg --multiline '(?s)Scanner.*def check$' ./modules/auxiliary

Example module which overrides the check which doesn't honor RHOSTS semantics:

https://github.com/rapid7/metasploit-framework/blob/3c0222d7d0cdde4ff8aebb74672624964988e038/modules/auxiliary/scanner/http/wp_chopslider_id_sqli.rb#L51-L65

Another egecase to consider; How should the AutoCheck functionality work with scanners?

• Run check against all targets first separately?
• Run check_host before each run_host

[adfoster-r7]

This would need more investigation cycles, there isn't a clear path forward without more research cycles

In particular, a note from @jmartin-r7 that scanner modules themselves are check implementations, so having a check implementation is also not quite semantically correct.

[jmartin-tech]

Scanner modules should look for a known vuln or mis-configuration on hosts they are targeted at. Since RHOSTS will now walk all targets for any module, a module that does more should likely be classified by what is does, gather, exploit, enumerate, etc and should likely no longer be a scanner.

Modules that do not reach arbitrary code execution to meet the bar for delivering a Metasploit payload but do exercise a vulnerability or mis-configuration are not scanners, however by implementing a check function any module can act as a scanner.

[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.