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

HTTP Crawler: don't expect page object for msg #16668

Merged

Conversation

sempervictus
Copy link
Contributor

The crawler_process_page method in HttpCrawler assumes that the
page object passed into the method is not nil when formatting the
msg string for printing to console.
Address the assumption with a ternary check leaving the || "ERR"
handling for page.code itself being nil inside the assignment
when page is not nil.

Testing:
Error accessing page undefined method '[]' for nil:NilClass is
no longer being thrown when scanning an odd HTTP service.

The `crawler_process_page` method in HttpCrawler assumes that the
`page` object passed into the method is not nil when formatting the
`msg` string for printing to console.
Address the assumption with a ternary check leaving the `|| "ERR"`
handling for `page.code` itself being nil inside the assignment
when page is not nil.

Testing:
 `Error accessing page undefined method '[]' for nil:NilClass` is
no longer being thrown when scanning an odd HTTP service.
@gwillcox-r7 gwillcox-r7 self-assigned this Jun 13, 2022
@gwillcox-r7
Copy link
Contributor

This actually is a bug in two places in the code as it seems that modules/auxiliary/scanner/http/crawler.rb actually overrides the definition at

#
# The main callback from the crawler, redefines crawler_process_page() as
# defined by Msf::Auxiliary::HttpCrawler
#
# Data we will report:
# - The path of any URL found by the crawler (web.uri, :path => page.path)
# - The occurence of any form (web.form :path, :type (get|post|path_info), :params)
#
def crawler_process_page(t, page, cnt)
and also has the same bug that is noted in the library.

@gwillcox-r7
Copy link
Contributor

Alright I think the best solution to this would be to add a guard clause. The change you implemented is also valid for checking page.code is not invalid so I'll keep that in.

@gwillcox-r7
Copy link
Contributor

This actually is a bug in two places in the code as it seems that modules/auxiliary/scanner/http/crawler.rb actually overrides the definition at

#
# The main callback from the crawler, redefines crawler_process_page() as
# defined by Msf::Auxiliary::HttpCrawler
#
# Data we will report:
# - The path of any URL found by the crawler (web.uri, :path => page.path)
# - The occurence of any form (web.form :path, :type (get|post|path_info), :params)
#
def crawler_process_page(t, page, cnt)

and also has the same bug that is noted in the library.

Fixed this in d20fa45

@gwillcox-r7 gwillcox-r7 added the rn-fix release notes fix label Jul 21, 2022
@gwillcox-r7 gwillcox-r7 merged commit abe90c1 into rapid7:master Jul 21, 2022
@gwillcox-r7
Copy link
Contributor

Release Notes

A bug has been fixed in the HTTP crawler module and its associated library whereby the code expected an object to be populated when it may not be. This has been fixed with additional validation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants