Add human-readable descriptions to CheckCode returns in modules#21353
Add human-readable descriptions to CheckCode returns in modules#21353adfoster-r7 wants to merge 1 commit intorapid7:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR standardizes Metasploit module check results by adding human-readable messages to CheckCode returns, improving how check outcomes bubble up to users (continuing work from #21304).
Changes:
- Add descriptive strings to
Exploit::CheckCode/CheckCodereturns across many exploit modules. - Refine some check outcome messaging (e.g., “detected but cannot confirm vulnerable”, “no response”, “not vulnerable”).
- Improve consistency of user-facing status explanations from
check.
Reviewed changes
Copilot reviewed 117 out of 117 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/exploits/multi/http/os_cmd_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/orientdb_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/oracle_reports_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/oracle_ebs_cve_2025_61882_exploit_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/oracle_ats_file_upload.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/oracle_access_manager_rce_cve_2021_35587.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/opmanager_sumpdu_deserialization.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/opmanager_socialit_file_upload.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/openx_backdoor_php.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/openmrs_deserialization.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/openfire_auth_bypass_rce_cve_2023_32315.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/openfire_auth_bypass.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/open_web_analytics_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/op5_welcome.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/op5_license.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/october_upload_bypass_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/nuuo_nvrmini_upgrade_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/novell_servicedesk_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/nostromo_code_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/nibbleblog_file_upload.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/netwin_surgeftp_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/navigate_cms_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/mybb_rce_cve_2022_24734.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/mutiny_subnetmask_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/movabletype_upgrade_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/monstra_fileupload_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/monitorr_webshell_rce_cve_2020_28871.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/mobilecartly_upload_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/mma_backdoor_upload.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/microfocus_ucmdb_unauth_deser.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/microfocus_obm_auth_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/metasploit_static_secret_key_base.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/mediawiki_thumb.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/mediawiki_syntaxhighlight.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/mantisbt_php_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/mantisbt_manage_proj_page_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/manageengine_servicedesk_plus_saml_rce_cve_2022_47966.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/manageengine_search_sqli.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/manageengine_sd_uploader.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/manageengine_auth_upload.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/manageengine_adselfservice_plus_saml_rce_cve_2022_47966.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/manage_engine_dc_pmp_sqli.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/makoserver_cmd_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/magento_unserialize.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/log1cms_ajax_create_folder.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/lighthouse_studio_unauth_rce_cve_2025_34300.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/lcms_php_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/kordil_edms_upload_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/kong_gateway_admin_api_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/joomla_http_header_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/jira_plugin_upload.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/jira_hipchat_template.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/jetbrains_teamcity_rce_cve_2024_27198.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/jetbrains_teamcity_rce_cve_2023_42793.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/jenkins_xstream_deserialize.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/jenkins_script_console.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/jenkins_metaprogramming.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/jboss_seam_upload_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/jboss_invoke_deploy.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/ibm_openadmin_tool_soap_welcomeserver_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/hyperic_hq_script_console.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/hp_sys_mgmt_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/hp_sitescope_issuesiebelcmd.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/glpi_install_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/glossword_upload_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/gitlist_arg_injection.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/gitlab_shell_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/gitlab_exif_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/gibbon_auth_rce_cve_2024_24725.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/getsimplecms_unauth_code_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/gambio_unauth_rce_cve_2024_23759.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/familycms_less_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/extplorer_upload_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/eventlog_file_upload.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/eaton_nsm_code_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/dotcms_file_upload_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/dexter_casinoloader_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/cve_2021_35464_forgerock_openam.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/cuteflow_upload_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/cups_bash_env_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/crushftp_rce_cve_2023_43177.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/connectwise_screenconnect_rce_cve_2024_1709.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/confluence_widget_connector.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/coldfusion_rds_auth_bypass.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/cockpit_cms_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/cmsms_upload_rename_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/cmsms_showtime2_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/cmsms_object_injection_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/cmsms_file_manager_auth_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/clipbucket_fileupload_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/clinic_pms_fileupload_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/cleo_rce_cve_2024_55956.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/cisco_dcnm_upload_2019.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/cisco_dcnm_upload.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/caidao_php_backdoor_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/cacti_pollers_sqli_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/cacti_package_import_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/cacti_graph_template_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/builderengine_upload_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/bolt_file_upload.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/bitbucket_env_var_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/bassmaster_js_injection.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/baldr_upload_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/auxilium_upload_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/atutor_sqli.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/atlassian_confluence_webwork_ognl_injection.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/atlassian_confluence_rce_cve_2023_22515.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/apprain_upload_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/apache_roller_ognl_injection.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/apache_mod_cgi_bash_env_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/apache_flink_jar_upload_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/apache_druid_cve_2023_25194.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/apache_apisix_api_default_token_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/ajaxplorer_checkinstall_exec.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/agent_tesla_panel_rce.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/adobe_coldfusion_rce_cve_2023_26360.rb | Add descriptive CheckCode messages in check |
| modules/exploits/multi/http/activecollab_chat.rb | Add descriptive CheckCode messages in check |
| return CheckCode::Unknown('No response received from the target') unless login_res.code == 200 | ||
|
|
||
| @session_id = get_sid(login_res) | ||
| @xsrf_token = login_res.get_html_document.at('meta[@id="atlassian-token"]') | ||
| return CheckCode::Unknown if @xsrf_token.nil? || @xsrf_token['content'].nil? | ||
| return CheckCode::Unknown('No response received from the target') if @xsrf_token.nil? || @xsrf_token['content'].nil? | ||
|
|
There was a problem hiding this comment.
Several CheckCode::Unknown('No response received from the target') returns here are triggered even when an HTTP response was received (non-200 login response, missing XSRF meta tag). Consider using more specific messages such as "Unexpected HTTP status" and "Missing XSRF token in response" to accurately reflect the failure mode.
| }) | ||
| return Exploit::CheckCode::Unknown unless res | ||
| return Exploit::CheckCode::Unknown('Received an unexpected response from the target') unless res | ||
| # the patched version will respond back with 200 OK and no data in the response body |
There was a problem hiding this comment.
This Unknown return is executed when res is nil (no response), but the message says "Received an unexpected response". Consider changing it to a no-response/connection-failed message so it matches the actual condition.
| unless res | ||
| return Exploit::CheckCode::Unknown # Target didn't respond | ||
| return Exploit::CheckCode::Unknown('Received an unexpected response from the target') # Target didn't respond | ||
| end |
There was a problem hiding this comment.
The new CheckCode::Unknown message here says "Received an unexpected response", but this branch is specifically when res is nil (no HTTP response). Consider changing the message to reflect an unreachable/timeout/no-response condition to match the inline comment ("Target didn't respond").
| if res.get_cookies.empty? | ||
| return Exploit::CheckCode::Unknown # Target didn't send us any cookies. We can't continue. | ||
| return Exploit::CheckCode::Unknown('No response received from the target') # Target didn't send us any cookies. We can't continue. | ||
| end | ||
|
|
||
| match = res.get_cookies.match(/([_A-Za-z0-9]+)=([A-Za-z0-9%]*)--([0-9A-Fa-f]+);/) | ||
|
|
||
| unless match | ||
| return Exploit::CheckCode::Unknown # Target didn't send us a session cookie. We can't continue. | ||
| return Exploit::CheckCode::Unknown('No response received from the target') # Target didn't send us a session cookie. We can't continue. | ||
| end |
There was a problem hiding this comment.
These branches return "No response received from the target", but they execute after receiving a response that either had no cookies or did not contain a session cookie. This message is misleading; consider using wording like "No cookies received" / "No session cookie found" (or similar) so users can distinguish it from a connectivity problem.
| if (!res) or (res.code != 200) | ||
| vprint_error("Unable to make a request to: #{path}") | ||
| return Exploit::CheckCode::Unknown | ||
| return Exploit::CheckCode::Unknown('Could not authenticate to the target') | ||
| end |
There was a problem hiding this comment.
This error message says "Could not authenticate to the target", but it is reached when the request fails entirely or returns a non-200 status from login.jsp. That is not necessarily an authentication failure. Consider a message that reflects request/HTTP failure (e.g., unable to fetch the login page / unexpected HTTP status).
| if versioncheck.nil? | ||
| vprint_error('Unable to detect Openfire version') | ||
| return Exploit::CheckCode::Unknown | ||
| return Exploit::CheckCode::Unknown('No response received from the target') | ||
| end |
There was a problem hiding this comment.
This branch occurs when a response body was received but the Openfire version regex did not match. Returning "No response received from the target" is inaccurate; consider wording like "Unable to determine Openfire version from response" so the failure mode is clear.
|
|
||
| if res and res.body =~ /Error during search/ | ||
| return Exploit::CheckCode::Vulnerable | ||
| return Exploit::CheckCode::Vulnerable('Successfully exploited SQL injection') | ||
| else | ||
| return Exploit::CheckCode::Safe | ||
| return Exploit::CheckCode::Safe('The target is not vulnerable') |
There was a problem hiding this comment.
The check only looks for an error pattern in the response to infer a SQL injection condition; it does not actually exploit anything. The message "Successfully exploited SQL injection" is misleading for a check result—consider changing it to indicate detection of a likely SQLi condition instead.
|
|
||
| return Exploit::CheckCode::Safe | ||
| return Exploit::CheckCode::Safe('The target is not vulnerable') | ||
| end |
There was a problem hiding this comment.
The final return Exploit::CheckCode::Safe(...) is unreachable because all paths above it return from the method (either Vulnerable or Safe). It should be removed to avoid dead code and to keep the check logic clear.
adfoster-r7
force-pushed
the
improve-checkcode-messages-6
branch
from
83a0748 to
f8c4099
Compare
| return Exploit::CheckCode::Unknown('Could not connect to the target') unless response | ||
| return Exploit::CheckCode::Appears('The target appears to be vulnerable based on the response') if response.code == 200 && response.body.match(content) | ||
| return Exploit::CheckCode::Detected('The target service was detected') if response.code == 200 | ||
|
|
||
| vprint_error("Server responded with: HTTP #{response.code}") | ||
| return Exploit::CheckCode::Safe | ||
| return Exploit::CheckCode::Safe('The target is not vulnerable') | ||
| end |
There was a problem hiding this comment.
CheckCode prepends a default message for Safe ("The target is not exploitable.") and then appends the provided reason (see Msf::Exploit::CheckCode#initialize). Using a reason like 'The target is not vulnerable' results in redundant output ("The target is not exploitable. The target is not vulnerable"). Consider using a more specific reason here (e.g., unexpected HTTP status, marker not reflected, etc.).
| else | ||
| vprint_status('Something went wrong, make sure host is up and options are correct!') | ||
| vprint_status("HTTP Response Code: #{res.code}") | ||
| return Exploit::CheckCode::Unknown | ||
| return Exploit::CheckCode::Unknown('No response received from the target') | ||
| end |
There was a problem hiding this comment.
In the non-200 response branch, the method returns Unknown('No response received from the target') even though res is present (you just printed res.code). This should use a reason that reflects the unexpected HTTP status / insufficient privileges rather than claiming no response.
| if response.redirect? && response.headers['location'] =~ /login.php/ && !(datastore['USERNAME'] && datastore['PASSWORD']) | ||
| print_warning('Unauthenticated RCE can\'t be exploited, retry if you gain CnC credentials.') | ||
| return Exploit::CheckCode::Unknown | ||
| return Exploit::CheckCode::Unknown('No response received from the target') |
There was a problem hiding this comment.
This branch handles an HTTP redirect to login.php when credentials are not supplied, but returns Unknown('No response received from the target') despite having a valid response. Consider returning Detected/Safe/Unknown with a reason indicating authentication is required or that unauthenticated exploitation isn't possible.
| return Exploit::CheckCode::Unknown('No response received from the target') | |
| return Exploit::CheckCode::Unknown('Target requires authentication; unauthenticated exploitation is not possible without valid CnC credentials') |
| else | ||
| print_status("Version: #{res.headers['Server']}") | ||
| return Exploit::CheckCode::Safe | ||
| return Exploit::CheckCode::Safe('The target is not running a vulnerable version') |
There was a problem hiding this comment.
In the else branch, print_status("Version: #{res.headers['Server']}") will raise if res is nil (e.g., connection failure), because the else also covers the res == nil case. Handle res.nil? separately and return an appropriate Unknown check code before dereferencing headers.
| openfire_version = get_version | ||
| return CheckCode::Safe if openfire_version.nil? | ||
| return CheckCode::Safe('The target is not running a vulnerable version') if openfire_version.nil? | ||
| # check first for patched versions |
There was a problem hiding this comment.
get_version returns nil both when the target isn't Openfire and when the request fails (no response / non-200). Returning CheckCode::Safe on a nil version can misclassify unreachable or unexpected targets as "not vulnerable"; this should be Unknown (or at least Detected) with a reason that version detection failed.
| return Exploit::CheckCode::Appears if version > Rex::Version.new('2') | ||
| return Exploit::CheckCode::Appears('The target is running a vulnerable version') if version > Rex::Version.new('2') | ||
| end | ||
| return Exploit::CheckCode::Safe('A vulnerable version if APISIX server is not running') |
There was a problem hiding this comment.
The Safe reason 'A vulnerable version if APISIX server is not running' is grammatically incorrect and semantically confusing (it reads like it’s safe because a vulnerable version isn’t running). Use a clearer reason such as “APISIX not detected in Server header” or “Server header did not match APISIX”.
| return Exploit::CheckCode::Safe('A vulnerable version if APISIX server is not running') | |
| return Exploit::CheckCode::Safe('APISIX not detected in Server header') |
| return CheckCode::Appears('The target appears to be vulnerable') if vulnerable_servlet_accessible?(csrf_token) | ||
|
|
||
| CheckCode::Safe | ||
| CheckCode::Safe('The target application was not detected') | ||
| end |
There was a problem hiding this comment.
CheckCode::Safe('The target application was not detected') is returned after oracle_ebs_detected? succeeded and the CSRF token was retrieved. This reason is misleading (the application was detected); it should instead explain why the target is considered safe (e.g., the vulnerable servlet was not accessible / exploit preconditions not met).
| major, minor, patch = get_version(response.body) | ||
|
|
||
| unless major | ||
| return CheckCode::Unknown | ||
| return CheckCode::Unknown('No response received from the target') | ||
| end |
There was a problem hiding this comment.
When get_version fails to extract a version from an existing HTTP response (major is nil), the code returns CheckCode::Unknown('No response received from the target'). This is factually incorrect because a response was received; use a reason that reflects the real failure (e.g., unable to determine MediaWiki version / missing generator tag).
| if res and res.code == 200 and res.headers['Server'] =~ /OrientDB Server v\.2\.2\./ | ||
| print_good("Version: #{res.headers['Server']}") | ||
| return Exploit::CheckCode::Vulnerable | ||
| return Exploit::CheckCode::Vulnerable('Successfully verified path traversal vulnerability') | ||
| else |
There was a problem hiding this comment.
The check condition only matches a Server header regex; the new reason 'Successfully verified path traversal vulnerability' is inaccurate for what was actually validated (and this module is RCE-oriented). Update the reason to describe the real evidence (e.g., OrientDB 2.2.x banner detected) or adjust the check code if you intend to assert exploitability.
| script = res.get_html_document.xpath('//script[contains(text(), "BUILD_NUMBER")]') | ||
| info = script.text.match(/PRODUCT_NAME\\x22\\x3A\\x22(?<product>.+?)\\x22,.*BUILD_NUMBER\\x22\\x3A\\x22(?<build>[0-9]+?)\\x22,/) | ||
| return CheckCode::Unknown unless info | ||
| return CheckCode::Unknown('No response received from the target') unless info | ||
| unless info[:product] == 'ManageEngine\\x20ServiceDesk\\x20Plus' |
There was a problem hiding this comment.
info being nil here means the build/product regex didn’t match the HTML, not that there was "No response". The current reason is misleading; update it to reflect parsing/extraction failure (e.g., unable to extract build number / product from response).
| unless res.headers['X-Powered-By'] | ||
| vprint_error("Unable to determine the PHP version.") | ||
| return Exploit::CheckCode::Unknown | ||
| return Exploit::CheckCode::Unknown('Could not connect to the target') | ||
| end |
There was a problem hiding this comment.
If the response lacks the X-Powered-By header, the code returns Unknown('Could not connect to the target'), but the request succeeded; the PHP version just couldn't be determined from headers. Adjust the message to reflect missing version info (or fallback to other detection), rather than a connectivity failure.
| res = send_request_raw({ 'uri' => "#{uri.path}listDatabases" }) | ||
| if res and res.code == 200 and res.headers['Server'] =~ /OrientDB Server v\.2\.2\./ | ||
| print_good("Version: #{res.headers['Server']}") | ||
| return Exploit::CheckCode::Vulnerable | ||
| return Exploit::CheckCode::Vulnerable('Successfully verified path traversal vulnerability') | ||
| else | ||
| print_status("Version: #{res.headers['Server']}") | ||
| return Exploit::CheckCode::Safe | ||
| return Exploit::CheckCode::Safe('The target is not running a vulnerable version') |
There was a problem hiding this comment.
check can raise when res is nil: the else branch still interpolates res.headers['Server']. Also, the new Vulnerable message mentions a path traversal, but this check only matches the OrientDB Server header and does not verify a path traversal condition. Consider guarding the else branch for nil responses (and returning Unknown), and adjusting the message (and possibly the check code) to reflect what was actually validated.
| unless res.body =~ /Mantis ([0-9]+\.[0-9]+\.[0-9]+)/ | ||
| vprint_error('Cannot determine Mantis version!') | ||
| return CheckCode::Unknown | ||
| return CheckCode::Unknown('Could not connect to the target') | ||
| end |
There was a problem hiding this comment.
If the login page response body doesn't match the version regex, the code returns Unknown('Could not connect to the target'), but connectivity succeeded—version detection failed. Update the message to reflect that the Mantis version could not be determined from the response.
| # default example app | ||
| if res and res.code == 200 and res.body =~ /#{rando}/ | ||
| return CheckCode::Vulnerable | ||
| return CheckCode::Vulnerable('Successfully verified path traversal vulnerability') |
There was a problem hiding this comment.
This module validates a JavaScript injection by checking for reflected content, but the new Vulnerable message says “path traversal vulnerability”. Update the message to match the actual condition being verified (JS injection / code execution), so users aren’t misled about the vulnerability class.
| return CheckCode::Vulnerable('Successfully verified path traversal vulnerability') | |
| return CheckCode::Vulnerable('Successfully verified JavaScript injection leading to code execution') |
adfoster-r7
force-pushed
the
improve-checkcode-messages-6
branch
2 times, most recently
from
9c980bf to
5ebeba1
Compare
| if res and res.code != 200 | ||
| vprint_error("Error accessing the index.php file") | ||
| return CheckCode::Unknown | ||
| return CheckCode::Unknown('Could not authenticate to the target') | ||
| end |
There was a problem hiding this comment.
If /admin/index.php responds with a non-200 (commonly a 302 redirect to login, or 404), the code returns Unknown('Could not authenticate to the target'). Since no authentication has been attempted yet, consider using a message that reflects the actual condition (e.g., authentication required / unexpected HTTP status).
|
|
||
| if res and res.code == 200 and res.body =~ /#{sum}/ | ||
| return Exploit::CheckCode::Vulnerable | ||
| return Exploit::CheckCode::Vulnerable('Successfully verified path traversal vulnerability') |
There was a problem hiding this comment.
This module is an OGNL injection issue, but the CheckCode::Vulnerable reason says “path traversal vulnerability”. Consider updating the message to reflect OGNL injection / code execution verification, to avoid misreporting the vulnerability type.
| return Exploit::CheckCode::Vulnerable('Successfully verified path traversal vulnerability') | |
| return Exploit::CheckCode::Vulnerable('Successfully verified OGNL injection vulnerability') |
| if (res.code == 200 and res.body =~ /Can't locate object method \\"dbi_driver\\" via package \\"#{fingerprint}\\" at/) | ||
| return Exploit::CheckCode::Vulnerable | ||
| return Exploit::CheckCode::Vulnerable('Successfully verified path traversal vulnerability') | ||
| elsif (res.code != 200) |
There was a problem hiding this comment.
The Vulnerable reason says “Successfully verified path traversal vulnerability”, but the module description and check logic indicate a Perl code injection via core_drop_meta_for_table / eval, not path traversal. Consider updating the message to match the actual vulnerability being tested.
| # the only way to test if the target is vuln | ||
| if test_injection | ||
| return Exploit::CheckCode::Vulnerable | ||
| return Exploit::CheckCode::Vulnerable('Successfully executed the injected code') |
There was a problem hiding this comment.
test_injection only verifies boolean-based SQL injection responses (it doesn’t execute code on the target). The CheckCode::Vulnerable reason “Successfully executed the injected code” is misleading here; consider wording it as “SQL injection confirmed” / “injection test succeeded”.
| return Exploit::CheckCode::Vulnerable('Successfully executed the injected code') | |
| return Exploit::CheckCode::Vulnerable('SQL injection confirmed') |
| elsif (res.code != 200) | ||
| return Exploit::CheckCode::Unknown | ||
| return Exploit::CheckCode::Unknown('Could not connect to the target') | ||
| else |
There was a problem hiding this comment.
In check, res.code != 200 returns Exploit::CheckCode::Unknown('Could not connect to the target'), but a non-200 HTTP response still indicates the connection succeeded. Consider returning Unknown with a message that reflects the unexpected HTTP status (or Safe if an HTTP response implies the target isn’t vulnerable).
| begin | ||
| res = send_request_cgi({ 'uri' => normalize_uri(target_uri.path, 'admin', 'index.php') }) | ||
| rescue | ||
| vprint_error("Unable to access the index.php file") | ||
| return CheckCode::Unknown | ||
| return CheckCode::Unknown('Could not authenticate to the target') | ||
| end |
There was a problem hiding this comment.
The rescue path returns CheckCode::Unknown('Could not authenticate to the target'), but this request is just a GET to /admin/index.php and can fail for many non-auth reasons (timeout, DNS, SSL, etc.). A more accurate message would describe the request failing/no response, and reserve “authenticate” wording for actual login attempts.
| res = send_request_cgi( | ||
| 'method' => 'GET', | ||
| 'uri' => normalize_uri(datastore['TARGETURI'], datastore['GUID']) | ||
| ) | ||
| return CheckCode::Unknown unless res | ||
| return CheckCode::Unknown('Could not authenticate to the target') unless res | ||
|
|
There was a problem hiding this comment.
return CheckCode::Unknown('Could not authenticate to the target') unless res is misleading here because the request is an unauthenticated GET to the SAML endpoint. If there’s no response, the message should reflect connectivity/no response rather than authentication.
| if res.body | ||
| if res.body.include?('Incorrect usage') | ||
| # We are able to determine that the server has a save.lsp page and | ||
| # returns the correct output. | ||
| vprint_status('Mako Server save.lsp returns correct ouput.') | ||
| return CheckCode::Appears | ||
| return CheckCode::Appears('The target appears vulnerable based on response headers') | ||
| else |
There was a problem hiding this comment.
This Appears reason says it’s based on “response headers”, but the decision is made from res.body.include?('Incorrect usage'). Consider updating the message to match what’s actually being checked (response body/content) to avoid confusing users.
| rescue ::Rex::ConnectionError => e | ||
| vprint_error(e.message) | ||
| return Exploit::CheckCode::Safe | ||
| return Exploit::CheckCode::Safe('The target is not vulnerable') | ||
| end |
There was a problem hiding this comment.
On Rex::ConnectionError, the method returns Exploit::CheckCode::Safe('The target is not vulnerable'). A connection error doesn’t establish safety; it should generally return Unknown (with the connection error) so automated workflows don’t treat unreachable targets as non-vulnerable.
| return CheckCode::Safe('Blind SQL injection test failed') unless sqli.test_vulnerable | ||
|
|
||
| CheckCode::Vulnerable | ||
| CheckCode::Vulnerable('Successfully executed the injected code') |
There was a problem hiding this comment.
The check returns CheckCode::Vulnerable('Successfully executed the injected code'), but at this point it has only verified that a time-based blind SQL injection is possible (sqli.test_vulnerable). Consider wording the reason as SQL injection confirmed (rather than code execution), since RCE hasn’t been demonstrated in check.
| CheckCode::Vulnerable('Successfully executed the injected code') | |
| CheckCode::Vulnerable('Blind SQL injection confirmed') |
adfoster-r7
force-pushed
the
improve-checkcode-messages-6
branch
from
5ebeba1 to
2d5b519
Compare
…ploit modules (A-O)
adfoster-r7
force-pushed
the
improve-checkcode-messages-6
branch
from
2d5b519 to
4b81bc9
Compare
| 'uri' => normalize_uri(datastore['TARGETURI']) | ||
| ) | ||
| return CheckCode::Unknown unless res | ||
| return CheckCode::Unknown('Received an unexpected response from the target') unless res |
There was a problem hiding this comment.
When res is nil, this branch indicates the target did not respond / the request failed. The current message says an "unexpected response" even though no response was received. Consider changing the reason string to reflect a missing response (or include connection failure details if available).
| # If we reach this point, we didn't find the service | ||
| return Exploit::CheckCode::Unknown | ||
| return Exploit::CheckCode::Unknown('No response received from the target') | ||
| end |
There was a problem hiding this comment.
The final return Exploit::CheckCode::Unknown('No response received from the target') is unreachable because every path above already returns. It should be removed, or the control flow should be refactored so a single, reachable fallback return is used (with an accurate message for the actual failure case).
|
|
||
| return Exploit::CheckCode::Unknown | ||
| return Exploit::CheckCode::Unknown('Could not connect to the target') | ||
| end |
There was a problem hiding this comment.
This Unknown('Could not connect to the target') is returned not only when the request fails, but also when the response is non-200 or doesn't contain the expected marker. The message should reflect the actual condition (e.g., "UCMDB not detected" or "Unexpected response") and ideally distinguish between no response vs. a non-matching response.
| # unfortunately could not find an easy way to detect the version running, even when auth | ||
| if res && res.code == 200 && res.body.include?('Login - Operations Bridge Manager') | ||
| return Exploit::CheckCode::Detected | ||
| return Exploit::CheckCode::Detected('The target application was detected but the version could not be confirmed as vulnerable') | ||
| end | ||
|
|
||
| return Exploit::CheckCode::Unknown | ||
| return Exploit::CheckCode::Unknown('Could not authenticate to the target') | ||
| end |
There was a problem hiding this comment.
The fallback Unknown('Could not authenticate to the target') can trigger when the target is not Operations Bridge Manager or when the request fails, not just when authentication fails. Consider changing the reason to something that matches the detection logic (e.g., "Operations Bridge Manager not detected" / "No response received") and only using an authentication-specific message when an auth failure is actually observed.
| if version <= 7.1 && version >= 6.5 | ||
| return Exploit::CheckCode::Appears | ||
| return Exploit::CheckCode::Appears('The target appears to be a vulnerable version') | ||
| elsif version > 7.1 | ||
| return Exploit::CheckCode::Safe | ||
| return Exploit::CheckCode::Safe('The target is not running a vulnerable version') | ||
| else | ||
| return Exploit::CheckCode::Unknown | ||
| return Exploit::CheckCode::Unknown('Could not authenticate to the target') | ||
| end |
There was a problem hiding this comment.
get_version returns a numeric version and this else branch is reached when the parsed version is outside the tested ranges (e.g., < 6.5). That condition is not an authentication failure, so the reason string "Could not authenticate to the target" is misleading. Consider changing it to reflect an unsupported/unknown version (or adjust the logic if this branch is meant to represent an auth failure).
| if phpsessid.nil? | ||
| print_error('Failed to retrieve PHPSESSID. Target may not be vulnerable.') | ||
| return CheckCode::Unknown | ||
| return CheckCode::Unknown('Could not connect to the target') |
There was a problem hiding this comment.
If the PHPSESSID cookie is missing from an otherwise valid redirect response, this is not a connectivity failure. The reason string "Could not connect to the target" is misleading here; consider changing it to indicate the session cookie could not be retrieved (or that the response was unexpected).
| return CheckCode::Unknown('Could not connect to the target') | |
| return CheckCode::Unknown('Failed to retrieve PHPSESSID from the target response') |
| if res_listing && res_listing.code == 200 && !res_listing.body.nil? && res_listing.body&.include?(dummy_filename) | ||
| vprint_good("File #{dummy_filename} found in /pms/user_images. Target is vulnerable!") | ||
| CheckCode::Vulnerable | ||
| CheckCode::Vulnerable('Successfully verified the upload vulnerability') | ||
| else | ||
| vprint_error("File #{dummy_filename} not found in /pms/user_images. Target may not be vulnerable.") | ||
| CheckCode::Unknown | ||
| CheckCode::Unknown('No response received from the target') | ||
| end |
There was a problem hiding this comment.
In this branch res_listing may be present but the uploaded file isn't found (e.g., directory listing disabled, different path, or not vulnerable). Returning Unknown('No response received from the target') is misleading because a response may have been received; consider returning Safe or Unknown with a reason that reflects "uploaded file not found in listing" / "listing inaccessible" depending on res_listing.
| if (res && res.headers.key?('MediaWiki-API-Error')) | ||
| if (res.headers['MediaWiki-API-Error'] == 'internal_api_error_MWException') | ||
| return Exploit::CheckCode::Appears | ||
| return Exploit::CheckCode::Appears('The target appears vulnerable based on response headers') | ||
| elsif (res.headers['MediaWiki-API-Error'] == 'readapidenied') | ||
| print_error("Login is required") | ||
| end | ||
|
|
||
| return Exploit::CheckCode::Unknown | ||
| return Exploit::CheckCode::Unknown('Could not authenticate to the target') | ||
| end |
There was a problem hiding this comment.
MediaWiki-API-Error can indicate many error conditions; this returns Unknown('Could not authenticate to the target') even when the header value is not readapidenied. Consider returning an auth-related message only for readapidenied, and otherwise include the actual API error value (or use a more general "API returned an error" message).
| csrf_token = retrieve_csrf_token | ||
| return CheckCode::Unknown unless csrf_token | ||
| return CheckCode::Unknown('Could not connect to the target') unless csrf_token | ||
|
|
||
| return CheckCode::Appears if vulnerable_servlet_accessible?(csrf_token) | ||
| return CheckCode::Appears('The target appears to be vulnerable') if vulnerable_servlet_accessible?(csrf_token) |
There was a problem hiding this comment.
retrieve_csrf_token can return nil for reasons other than connectivity (e.g., token parsing failed / unexpected response). The message "Could not connect to the target" is therefore misleading here; consider using a reason like "Failed to retrieve CSRF token" and, if possible, distinguishing no-response from parse/HTTP errors inside retrieve_csrf_token.
| version = get_mantis_version | ||
|
|
||
| return Exploit::CheckCode::Unknown if version.nil? | ||
| return Exploit::CheckCode::Unknown('No response received from the target') if version.nil? | ||
|
|
There was a problem hiding this comment.
get_mantis_version returns nil not only when the target doesn't respond, but also when the response is non-200 or when the version regex doesn't match (it even prints "Can not detect Mantis version"). The reason string "No response received from the target" is misleading here; consider changing it to something like "Unable to detect Mantis version" (and optionally distinguish no-response vs parse failure if needed).
Improves multiple module check code messages and statuses
This metadata is currently missing in modules, which means the bubbling up of results to users is often missing
Continuation of #21304
Verification