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

Bugfix Msf::Exploit::FileDropper.file_dropper_exist? for Windows sessions #18844

Merged
merged 2 commits into from Mar 5, 2024

Conversation

sfewer-r7
Copy link
Contributor

@sfewer-r7 sfewer-r7 commented Feb 16, 2024

When using the FileDropper mixin for a shell session on the windows platform, it appears the function file_dropper_exist? may have been broken for ~9 years, preventing files and directories from being deleted if they exist. The function shell_command_token was being called, but it should be session.shell_command_token.

      if session.platform == 'windows'
        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}\" -o -d \"#{normalized}\" && echo true")
      end

An exception was being thrown when the function shell_command_token could not be found, and swallowed (silently unless you have debug logging enabled) in Msf::Payload::on_session:

  def on_session(session)

    # If this payload is associated with an exploit, inform the exploit
    # that a session has been created and potentially shut down any
    # open sockets. This allows active exploits to continue hammering
    # on a service until a session is created.
    if (assoc_exploit)

      # Signal that a new session is created by calling the exploit's
      # on_new_session handler. The default behavior is to set an
      # instance variable, which the exploit will have to check.
      begin
        assoc_exploit.on_new_session(session)
      rescue ::Exception => e
        dlog("#{assoc_exploit.refname}: on_new_session handler triggered exception: #{e.class} #{e} #{e.backtrace}", 'core', LEV_1)	rescue nil
      end

Additionally, the logic in the file_dropper_exist? function for windows shell sessions is wrong. The function should test if a path is either a file or a directory, the logic current only tests is a path is a file and return false if it is a directory. This does not match how the test is done on either meterpreter sessions or non windows shell sessions. (I think the code was copied from the File post modules back when FileDropper only supported cleaning up files, but now FileDropper supports directories, so file_dropper_exist? must test for either. the Windows shell command IF EXIST will test for both implicitly)

So with the 2 fixes mentioned in this pull request, the following works as expected in my testing:

      if session.platform == 'windows'
        f = session.shell_command_token("cmd.exe /C IF exist \"#{normalized}\" ( echo true )")
      else
        f = session.shell_command_token("test -f \"#{normalized}\" -o -d \"#{normalized}\" && echo true")
      end

…stered for cleanup were not being deleted. We must call session.shell_command_token
…irectory, the logic for shell sessions on wqindows is testing if a path if a file and not a directory. this is wrong. Origionally FileDropper only supported cleaningup files, so this logic made sense (it was copied over from teh File post moduile) but FileDropper has since supported directories so teh logic here neds to reflect that.
@smcintyre-r7 smcintyre-r7 self-assigned this Mar 5, 2024
@smcintyre-r7
Copy link
Contributor

The code here all looks correct to me. I wrote up a little module to test this in isolation and using that module I was able to reproduce the original issue and validate that these changes fix it.

test/file_dropper.rb
##
# This module requires Metasploit: https://metasploit.com/download
# Current source: https://github.com/rapid7/metasploit-framework
##

class MetasploitModule < Msf::Post
  include Msf::ModuleTest::PostTest
  include Msf::ModuleTest::PostTestFileSystem
  include Msf::Post::Common
  include Msf::Post::File
  include Msf::Exploit::FileDropper

  def initialize(info = {})
    super(
      update_info(
        info,
        'Name' => '',
        'Description' => %q{

        },
        'References' => [
        ],
        'DisclosureDate' => '2024-03-04',
        'License' => MSF_LICENSE,
        'Author' => [
          'Spencer McIntyre'
        ],
        'Platform' => [ 'win'],
        'Targets' => [ ['Windows', {}] ],
        'SessionTypes' => [ 'meterpreter', 'shell' ],
        'DefaultTarget' => 0,
        'Notes' => {
          'SideEffects' => [ ARTIFACTS_ON_DISK ],
        }
      )
    )
  end

  def setup
    @bin_path = datastore['BaseFileName']
    print_status("Writing #{@bin_path} to disk")
    write_file(@bin_path, Rex::Text.rand_text_alphanumeric(100))
    register_file_for_cleanup(@bin_path)
  end

  def test_file_dropper_delete_file
    it 'should remove the file' do
      file_dropper_delete_file(session, @bin_path)
      !file?(@bin_path)
    end
  end

  def test_file_dropper_exist?
    it 'should tell that the file exists' do
      file_dropper_exist?(session, @bin_path)
    end
  end
end

@smcintyre-r7 smcintyre-r7 added library bug rn-fix release notes fix labels Mar 5, 2024
@smcintyre-r7 smcintyre-r7 merged commit b30f264 into rapid7:master Mar 5, 2024
52 checks passed
@smcintyre-r7
Copy link
Contributor

Release Notes

This fixes a bug in the file dropper mixin that would prevent files from being deleted with a Windows shell session.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug library rn-fix release notes fix
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants