rapid7 / rex-core

Created by David Maloney via the GitHub Connector
Other
4 stars 23 forks source link

Read/Write lock thread safety issue #27

Open adfoster-r7 opened 2 years ago

adfoster-r7 commented 2 years ago

It looks like the Reader/Writer lock isn't thread safe. I haven't investigated the issue further, as there's been no previously raised issues for this in metasploit-framework.

Simple resource script which reads/writes to a file on disk:

<ruby>

# results.txt will be used as an instance counter
# It will be incremented by 1 within a writer lock, and read via reader lock
File.write('./results.txt', '0', mode: 'w')
lock = ::Rex::ReadWriteLock.new

begin
  threads = 100.times.map do
    Thread.new do
      300.times do
        if rand > 0.5
          lock.synchronize_write do
            value = File.read('./results.txt').to_i
            File.write('./results.txt', value + 1, mode: 'w')
          end
        end

        lock.synchronize_read do
          value = File.read('./results.txt')
        end
      end
    end
  end

  threads.each(&:join)
ensure
  puts "File result: #{File.read('./results.txt')}"
end

</ruby>

Exception:

msf6 exploit(multi/script/web_delivery) > resource resource.rc
[*] Processing /Users/user/Documents/code/metasploit-framework/resource.rc for ERB directives.
[*] resource (/Users/user/Documents/code/metasploit-framework/resource.rc)> Ruby Code (791 bytes)
File result: 
[-] resource (/Users/user/Documents/code/metasploit-framework/resource.rc)> Ruby Error: ThreadError Attempt to unlock a mutex which is not locked [
    "/Users/user/.rvm/gems/ruby-3.0.2@metasploit-framework/gems/rex-core-0.1.26/lib/rex/sync/read_write_lock.rb:107:in `unlock'",
    "/Users/user/.rvm/gems/ruby-3.0.2@metasploit-framework/gems/rex-core-0.1.26/lib/rex/sync/read_write_lock.rb:107:in `ensure in unlock_read'",
    "/Users/user/.rvm/gems/ruby-3.0.2@metasploit-framework/gems/rex-core-0.1.26/lib/rex/sync/read_write_lock.rb:107:in `unlock_read'",
    "/Users/user/.rvm/gems/ruby-3.0.2@metasploit-framework/gems/rex-core-0.1.26/lib/rex/sync/read_write_lock.rb:152:in `synchronize_read'",
    "(eval):18:in `block (3 levels) in load_resource'",
    "(eval):10:in `times'", "(eval):10:in `block (2 levels) in load_resource'",
    "/Users/user/.rvm/gems/ruby-3.0.2@metasploit-framework/gems/logging-2.3.0/lib/logging/diagnostic_context.rb:474:in `block in create_with_logging_context'"
]
sempervictus commented 2 years ago

Yike, i think i've seen this before... when i was trying to get MSF ported over to Rubinius many moons ago. That branch is a nightmare of Actors and Celluloid attempts, but i think i actually did something about this (gonna try to see what it was and if it was remotely sane).