Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FileDropper hangs indefinitely when trying to delete a file in use #15992

Closed
wvu opened this issue Dec 23, 2021 · 9 comments · Fixed by #17072
Closed

FileDropper hangs indefinitely when trying to delete a file in use #15992

wvu opened this issue Dec 23, 2021 · 9 comments · Fixed by #17072
Assignees
Labels
bug confirmed Issues confirmed by a committer library

Comments

@wvu
Copy link
Contributor

wvu commented Dec 23, 2021

Observed while testing #15950. Might be a regression, but I haven't had much time to debug.

@gwillcox-r7
Copy link
Contributor

@wvu-r7 If you can paste some details on what you did to cause this issue I'd be more than happy to take a look into this and try get a fix in place 👍

@wvu
Copy link
Contributor Author

wvu commented Dec 31, 2021

I had used FileDropper in #15950. I can post a diff in a bit. That'll be the easiest way to test.

@gwillcox-r7 gwillcox-r7 self-assigned this Dec 31, 2021
@gwillcox-r7 gwillcox-r7 added the confirmed Issues confirmed by a committer label Dec 31, 2021
@wvu
Copy link
Contributor Author

wvu commented Jan 13, 2022

This should do for replicating the bug:

wvu@kharak:~/rapid7/metasploit-framework:master$ git diff
diff --git a/modules/exploits/windows/http/manageengine_servicedesk_plus_cve_2021_44077.rb b/modules/exploits/windows/http/manageengine_servicedesk_plus_cve_2021_44077.rb
index a8566331ed..d2a21d5a10 100644
--- a/modules/exploits/windows/http/manageengine_servicedesk_plus_cve_2021_44077.rb
+++ b/modules/exploits/windows/http/manageengine_servicedesk_plus_cve_2021_44077.rb
@@ -10,6 +10,7 @@ class MetasploitModule < Msf::Exploit::Remote
   prepend Msf::Exploit::Remote::AutoCheck
   include Msf::Exploit::Remote::HttpClient
   include Msf::Exploit::EXE
+  include Msf::Exploit::FileDropper

   def initialize(info = {})
     super(
@@ -106,6 +107,9 @@ class MetasploitModule < Msf::Exploit::Remote
       fail_with(Failure::NotVulnerable, 'Failed to upload msiexec.exe')
     end

+    # Working directory is C:\Program Files\ManageEngine\ServiceDesk\site24x7
+    register_file_for_cleanup('..\\bin\\msiexec.exe')
+
     print_good('Successfully uploaded msiexec.exe')
   end

@@ -122,12 +126,4 @@ class MetasploitModule < Msf::Exploit::Remote
     }, 0)
   end

-  # XXX: FileDropper dies a miserable death if the file is in use
-  def on_new_session(_session)
-    super
-
-    # Working directory is C:\Program Files\ManageEngine\ServiceDesk\site24x7
-    print_warning("Yo, don't forget to clean up ..\\bin\\msiexec.exe")
-  end
-
 end
wvu@kharak:~/rapid7/metasploit-framework:master$

Untested against current code.

@space-r7
Copy link
Contributor

space-r7 commented Aug 9, 2022

In the file dropper mixin, the following line times out when attempting to delete a file:

session.shell_command_token(%Q|attrib.exe -r #{win_dir}|)

It looks like the regression was introduced in commit 4f8e435 with the addition of set_shell_token_index() to shell_command_token_win32(). The addition somehow results in the following shell_write() call timing out:

shell_write(cmd + "&echo #{token}#{command_termination}")

Reverting the shell_command_token_win32() method back to what it was prior to that commit fixes it, but I imagine that would result in a regression somewhere else. I was curious if set_shell_token_index() was meant for only Unix / Linux, as it uses ; as a command separator and won't execute both echo statements properly on Windows.

def set_shell_token_index(timeout)
return @shell_token_index if @shell_token_index
token = ::Rex::Text.rand_text_alpha(32)
numeric_token = rand(0xffffffff) + 1
cmd = "echo #{numeric_token}"
shell_write(cmd + ";echo #{token}#{command_termination}")
res = shell_read_until_token(token, 0, timeout)
if res.to_i == numeric_token
@shell_token_index = 0
else
@shell_token_index = 1
end
end

Because the two echo commands are only executed as one statement, the length of the parts array doesn't match parts_needed variable (70-71 in shell_read_until_token()), which eventually leads to the Timeout block raising Timeout::Error.

# Read until we get the data between two tokens or absolute timeout.
begin
::Timeout.timeout(timeout) do
buf = ''
idx = nil
loop do
if (tmp = shell_read(-1))
buf << tmp
# see if we have the wanted idx
unless buf.nil?
# normalize the line endings following the token and parse them
buf.gsub!("#{token}\n", "#{token}\r\n")
parts = buf.split("#{token}\r\n", -1)
if parts.length == parts_needed
# cause another prompt to appear (just in case)
shell_write(command_termination)
return parts[wanted_idx]
end
end
end
end
end
rescue Timeout::Error
# This is expected in many cases
end
# failed to get any data or find the token!
nil
end

I initially thought that the issue might be with the Timeout block itself after reading a few articles on it, but removing it and breaking the inner loop on some arbitrary condition still resulted in the shell_write() call in shell_command_token_win32() timing out. Also modified the command_termination() to write "\r\n" in case the platform is Windows, but that didn't fix the issue either. Not sure if I'm completely missing something here, but I'm pretty puzzled. Backtrace from the Timeout block posted below.

Backtrace
["/Users/space/rapid7/metasploit-framework/lib/rex/post/meterpreter/packet_response_waiter.rb:95:in `sleep'",
 "/Users/space/rapid7/metasploit-framework/lib/rex/post/meterpreter/packet_response_waiter.rb:95:in `wait'",
 "/Users/space/rapid7/metasploit-framework/lib/rex/post/meterpreter/packet_response_waiter.rb:95:in `block in wait'",
 "/Users/space/rapid7/metasploit-framework/lib/rex/post/meterpreter/packet_response_waiter.rb:93:in `synchronize'",
 "/Users/space/rapid7/metasploit-framework/lib/rex/post/meterpreter/packet_response_waiter.rb:93:in `wait'",
 "/Users/space/rapid7/metasploit-framework/lib/rex/post/meterpreter/packet_dispatcher.rb:234:in `send_packet_wait_response'",
 "/Users/space/rapid7/metasploit-framework/lib/rex/post/meterpreter/packet_dispatcher.rb:176:in `send_request'",
 "/Users/space/rapid7/metasploit-framework/lib/rex/post/meterpreter/channel.rb:195:in `_read'",
 "/Users/space/rapid7/metasploit-framework/lib/rex/post/meterpreter/channel.rb:172:in `read'",
 "/Users/space/rapid7/metasploit-framework/lib/rex/post/meterpreter/channels/pool.rb:77:in `read'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/base/sessions/meterpreter.rb:221:in `block in shell_read'",
 "/Users/space/.rvm/gems/ruby-3.0.2@metasploit-framework/gems/timeout-0.3.0/lib/timeout.rb:179:in `block in timeout'",
 "/Users/space/.rvm/gems/ruby-3.0.2@metasploit-framework/gems/timeout-0.3.0/lib/timeout.rb:36:in `block in catch'",
 "/Users/space/.rvm/gems/ruby-3.0.2@metasploit-framework/gems/timeout-0.3.0/lib/timeout.rb:36:in `catch'",
 "/Users/space/.rvm/gems/ruby-3.0.2@metasploit-framework/gems/timeout-0.3.0/lib/timeout.rb:36:in `catch'",
 "/Users/space/.rvm/gems/ruby-3.0.2@metasploit-framework/gems/timeout-0.3.0/lib/timeout.rb:188:in `timeout'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/base/sessions/meterpreter.rb:220:in `shell_read'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/core/session/provider/single_command_shell.rb:64:in `block (2 levels) in shell_read_until_token'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/core/session/provider/single_command_shell.rb:63:in `loop'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/core/session/provider/single_command_shell.rb:63:in `block in shell_read_until_token'",
 "/Users/space/.rvm/gems/ruby-3.0.2@metasploit-framework/gems/timeout-0.3.0/lib/timeout.rb:179:in `block in timeout'",
 "/Users/space/.rvm/gems/ruby-3.0.2@metasploit-framework/gems/timeout-0.3.0/lib/timeout.rb:36:in `block in catch'",
 "/Users/space/.rvm/gems/ruby-3.0.2@metasploit-framework/gems/timeout-0.3.0/lib/timeout.rb:36:in `catch'",
 "/Users/space/.rvm/gems/ruby-3.0.2@metasploit-framework/gems/timeout-0.3.0/lib/timeout.rb:36:in `catch'",
 "/Users/space/.rvm/gems/ruby-3.0.2@metasploit-framework/gems/timeout-0.3.0/lib/timeout.rb:188:in `timeout'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/core/session/provider/single_command_shell.rb:60:in `shell_read_until_token'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/core/session/provider/single_command_shell.rb:120:in `set_shell_token_index'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/core/session/provider/single_command_shell.rb:136:in `shell_command_token_win32'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/core/session/provider/single_command_shell.rb:90:in `shell_command_token'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/core/exploit/file_dropper.rb:203:in `file_dropper_delete_file'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/core/exploit/file_dropper.rb:85:in `block in on_new_session'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/core/exploit/file_dropper.rb:82:in `delete_if'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/core/exploit/file_dropper.rb:82:in `on_new_session'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/core/payload.rb:497:in `on_session'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/core/handler.rb:282:in `register_session'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/core/handler.rb:251:in `block in create_session'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/core/session_manager.rb:160:in `block (2 levels) in initialize_scheduler_threads'",
 "/Users/space/rapid7/metasploit-framework/lib/msf/core/thread_manager.rb:105:in `block in spawn'",
 "/Users/space/.rvm/gems/ruby-3.0.2@metasploit-framework/gems/logging-2.3.1/lib/logging/diagnostic_context.rb:474:in `block in create_with_logging_context'"]

@wvu
Copy link
Contributor Author

wvu commented Aug 10, 2022

Thank you, @space-r7!

@cdelafuente-r7
Copy link
Contributor

I did some additional tests and I found out the issue might be due to the repeated call to shell_read(-1) in shell_read_until_token, which occurs due the length of the parts array not matching parts_needed, as @space-r7 mentioned in the previous comment.

This call ends up timing out and returns nil since nothing is to be read anymore.

However, as mentioned in a comment in shell_read:

# Meterpreter doesn't offer a way to timeout on the victim side, so
# we have to do it here. I'm concerned that this will cause loss
# of data.
Timeout.timeout(timeout) {
rv = @shell.channel.read(length)
}

The timeout expires locally and not on the victim side. When the method returns due to a timeout, the process on the remote host is still trying to read something. This seems to block further write commands until the process on the victim side actually times out at some point.

This can be demonstrated by adding a breakpoint in set_shell_token_index and executing shell_read(-1) twice:

[14] pry(#<Msf::Sessions::Meterpreter_x64_Win>)> shell_write('echo test')
=> 10
[15] pry(#<Msf::Sessions::Meterpreter_x64_Win>)> puts shell_read(-1)
echo test
test

C:\Program Files (x86)\iView\Apache Software Foundation\Tomcat6.0>
=> nil
[16] pry(#<Msf::Sessions::Meterpreter_x64_Win>)> puts shell_read(-1) # will timeout since there is nothing to read

=> nil
[17] pry(#<Msf::Sessions::Meterpreter_x64_Win>)> shell_write('echo test') # blocks for a very long time

Whereas, calling shell_read(-1) just once doesn't block:

[1] pry(#<Msf::Sessions::Meterpreter_x64_Win>)> shell_write('echo test')
=> 10
[2] pry(#<Msf::Sessions::Meterpreter_x64_Win>)> puts shell_read(-1)

(c) 2018 Microsoft Corporation. All rights reserved.

C:\Program Files (x86)\iView\Apache Software Foundation\Tomcat6.0>echo test
test

C:\Program Files (x86)\iView\Apache Software Foundation\Tomcat6.0>
=> nil
[3] pry(#<Msf::Sessions::Meterpreter_x64_Win>)> shell_write('echo test')
=> 10
[4] pry(#<Msf::Sessions::Meterpreter_x64_Win>)> puts shell_read(-1)
echo test
test

C:\Program Files (x86)\iView\Apache Software Foundation\Tomcat6.0>
=> nil

So, I'm not sure where to fix this: removing the extra calls to shell_read(-1)? or addressing the blocking issue on the payload side?
Note that we can avoid calling shell_read(-1) multiple times by calling shell_read_until_token with wanted_idx set to 1 instead of 0 here:

res = shell_read_until_token(token, 0, timeout)

    res = shell_read_until_token(token, 1, timeout)

But it doesn't look like it is enough since shell_read_until_token is also called later with wanted_idx set to 0 and the process will block the same way.

@wvu
Copy link
Contributor Author

wvu commented Aug 17, 2022

Thank you, @cdelafuente-r7!

@gwillcox-r7 gwillcox-r7 removed their assignment Aug 17, 2022
@gwillcox-r7
Copy link
Contributor

Clearing my assignment since Christophe and Shelby seem to have more of a handle on this than I do. Feel free to pick this up 👍

@wvu
Copy link
Contributor Author

wvu commented Aug 17, 2022

I miss y'all!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug confirmed Issues confirmed by a committer library
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants