Land #5371, Add file checking to the on_new_session cleanup

lib/msf/core/exploit/file_dropper.rb

@@ -8,64 +8,10 @@ def initialize(info = {})
 
     register_advanced_options(
       [
-        OptInt.new( 'FileDropperDelay', [ false, 'Delay in seconds before attempting file cleanup' ])
+        OptInt.new('FileDropperDelay', [false, 'Delay in seconds before attempting file cleanup'])
       ], self.class)
   end
 
-  #
-  # When a new session is created, attempt to delete any files that the
-  # exploit created.
-  #
-  # @param (see Msf::Exploit#on_new_session)
-  # @return [void]
-  #
-  def on_new_session(session)
-    super
-
-    if session.type == "meterpreter"
-      session.core.use("stdapi") unless session.ext.aliases.include?("stdapi")
-    end
-
-    if not @dropped_files or @dropped_files.empty?
-      return true
-    end
-
-    @dropped_files.delete_if do |file|
-      win_file = file.gsub("/", "\\\\")
-      if session.type == "meterpreter"
-        begin
-          # Meterpreter should do this automatically as part of
-          # fs.file.rm().  Until that has been implemented, remove the
-          # read-only flag with a command.
-          if session.platform =~ /win/
-            session.shell_command_token(%Q|attrib.exe -r #{win_file}|)
-          end
-          session.fs.file.rm(file)
-          print_good("Deleted #{file}")
-          true
-        rescue ::Rex::Post::Meterpreter::RequestError
-          false
-        end
-      else
-        win_cmds = [
-            %Q|attrib.exe -r "#{win_file}"|,
-            %Q|del.exe /f /q "#{win_file}"|
-        ]
-        # We need to be platform-independent here. Since we can't be
-        # certain that {#target} is accurate because exploits with
-        # automatic targets frequently change it, we just go ahead and
-        # run both a windows and a unix command in the same line. One
-        # of them will definitely fail and the other will probably
-        # succeed. Doing it this way saves us an extra round-trip.
-        # Trick shared by @mihi42
-        session.shell_command_token("rm -f \"#{file}\" >/dev/null ; echo ' & #{win_cmds.join(" & ")} & echo \" ' >/dev/null")
-        print_good("Deleted #{file}")
-        true
-      end
-    end
-  end
-
-  #
   # Record file as needing to be cleaned up
   #
   # @param files [Array<String>] List of paths on the target that should
@@ -84,7 +30,32 @@ def register_files_for_cleanup(*files)
   # Singular version
   alias register_file_for_cleanup register_files_for_cleanup
 
+  # When a new session is created, attempt to delete any files that the
+  # exploit created.
   #
+  # @param (see Msf::Exploit#on_new_session)
+  # @return [void]
+  def on_new_session(session)
+    super
+
+    if session.type == 'meterpreter'
+      session.core.use('stdapi') unless session.ext.aliases.include?('stdapi')
+    end
+
+    unless @dropped_files && @dropped_files.length > 0
+      return
+    end
+
+    @dropped_files.delete_if do |file|
+      exists_before = file_dropper_file_exist?(session, file)
+      if file_dropper_delete(session, file)
+        file_dropper_deleted?(session, file, exists_before)
+      else
+        false
+      end
+    end
+  end
+
   # While the exploit cleanup do a last attempt to delete any files created
   # if there is a file_rm method available. Warn the user if any files were
   # not cleaned up.
@@ -109,9 +80,8 @@ def cleanup
       @dropped_files.delete_if do |file|
         begin
           file_rm(file)
-          print_good("Deleted #{file}")
-          true
-        #rescue ::Rex::SocketError, ::EOFError, ::IOError, ::Errno::EPIPE, ::Rex::Post::Meterpreter::RequestError => e
+          # We don't know for sure if file has been deleted, so always warn about it to the user
+          false
         rescue ::Exception => e
           vprint_error("Failed to delete #{file}: #{e}")
           elog("Failed to delete #{file}: #{e.class}: #{e}")
@@ -126,5 +96,101 @@ def cleanup
     end
 
   end
+
+  private
+
+  # See if +path+ exists on the remote system and is a regular file
+  #
+  # @param path [String] Remote filename to check
+  # @return [Boolean] True if the file exists, otherwise false.
+  def file_dropper_file_exist?(session, path)
+    if session.platform =~ /win/
+      normalized = file_dropper_win_file(path)
+    else
+      normalized = path
+    end
+
+    if session.type == 'meterpreter'
+      stat = session.fs.file.stat(normalized) rescue nil
+      return false unless stat
+      stat.file?
+    else
+      if session.platform =~ /win/
+        f = shell_command_token("cmd.exe /C IF exist \"#{normalized}\" ( echo true )")
+        if f =~ /true/
+          f = shell_command_token("cmd.exe /C IF exist \"#{normalized}\\\\\" ( echo false ) ELSE ( echo true )")
+        end
+      else
+        f = session.shell_command_token("test -f \"#{normalized}\" && echo true")
+      end
+
+      return false if f.nil? || f.empty?
+      return false unless f =~ /true/
+      true
+    end
+  end
+
+  # Sends a file deletion command to the remote +session+
+  #
+  # @param [String] file The file to delete
+  # @return [Boolean] True if the delete command has been executed in the remote machine, otherwise false.
+  def file_dropper_delete(session, file)
+    win_file = file_dropper_win_file(file)
+
+    if session.type == 'meterpreter'
+      begin
+        # Meterpreter should do this automatically as part of
+        # fs.file.rm().  Until that has been implemented, remove the
+        # read-only flag with a command.
+        if session.platform =~ /win/
+          session.shell_command_token(%Q|attrib.exe -r #{win_file}|)
+        end
+        session.fs.file.rm(file)
+        true
+      rescue ::Rex::Post::Meterpreter::RequestError
+        false
+      end
+    else
+      win_cmds = [
+        %Q|attrib.exe -r "#{win_file}"|,
+        %Q|del.exe /f /q "#{win_file}"|
+      ]
+      # We need to be platform-independent here. Since we can't be
+      # certain that {#target} is accurate because exploits with
+      # automatic targets frequently change it, we just go ahead and
+      # run both a windows and a unix command in the same line. One
+      # of them will definitely fail and the other will probably
+      # succeed. Doing it this way saves us an extra round-trip.
+      # Trick shared by @mihi42
+      session.shell_command_token("rm -f \"#{file}\" >/dev/null ; echo ' & #{win_cmds.join(" & ")} & echo \" ' >/dev/null")
+      true
+    end
+  end
+
+  # Checks if a file has been deleted by the current job
+  #
+  # @param [String] file The file to check
+  # @return [Boolean] If the file has been deleted, otherwise false.
+  def file_dropper_deleted?(session, file, exists_before)
+    if exists_before  && file_dropper_file_exist?(session, file)
+      print_error("Unable to delete #{file}")
+      false
+    elsif exists_before
+      print_good("Deleted #{file}")
+      true
+    else
+      print_warning("Tried to delete #{file}, unknown result")
+      true
+    end
+  end
+
+  # Converts a file path to use the windows separator '\'
+  #
+  # @param [String] file The file path to convert
+  # @return [String] The file path converted
+  def file_dropper_win_file(file)
+    file.gsub('/', '\\\\')
+  end
+
 end
 end

modules/exploits/multi/http/struts_code_exec_classloader.rb

@@ -271,9 +271,7 @@ def class_loader_exploit
       fail_with(Failure::Unknown, "#{peer} - The log file hasn't been flushed")
     end
 
-    # This path depends on CWD. May require manual cleanup
-    # See https://github.com/rapid7/metasploit-framework/issues/4667
-    print_warning("This exploit requires manual cleanup of '#{@jsp_file}' on the target")
+    register_files_for_cleanup(@jsp_file)
 
     # Prepare the JSP
     print_status("#{peer} - Generating JSP...")