From 96990fdc02aefe4f27b547d3607f4e6cce225ff4 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Wed, 5 Nov 2014 14:38:43 -0600 Subject: [PATCH 01/34] Fail before suite if more than 1 thread exists MSP-11147 Detect thread leaks in a `before(:suite)` configured by `Metasploit::Framework::Spec::Threads::Suite.configure!` and fail if any leaks are found. --- lib/metasploit/framework.rb | 4 +++ lib/metasploit/framework/spec.rb | 5 +++ lib/metasploit/framework/spec/threads.rb | 5 +++ .../framework/spec/threads/suite.rb | 35 +++++++++++++++++++ spec/spec_helper.rb | 2 ++ 5 files changed, 51 insertions(+) create mode 100644 lib/metasploit/framework/spec.rb create mode 100644 lib/metasploit/framework/spec/threads.rb create mode 100644 lib/metasploit/framework/spec/threads/suite.rb diff --git a/lib/metasploit/framework.rb b/lib/metasploit/framework.rb index aa488aea2a..a43afdc93c 100644 --- a/lib/metasploit/framework.rb +++ b/lib/metasploit/framework.rb @@ -32,6 +32,10 @@ module Metasploit # works in compatible manner with activerecord's rake tasks and other # railties. module Framework + extend ActiveSupport::Autoload + + autoload :Spec + # Returns the root of the metasploit-framework project. Use in place of # `Rails.root`. # diff --git a/lib/metasploit/framework/spec.rb b/lib/metasploit/framework/spec.rb new file mode 100644 index 0000000000..857c219d43 --- /dev/null +++ b/lib/metasploit/framework/spec.rb @@ -0,0 +1,5 @@ +module Metasploit::Framework::Spec + extend ActiveSupport::Autoload + + autoload :Threads +end \ No newline at end of file diff --git a/lib/metasploit/framework/spec/threads.rb b/lib/metasploit/framework/spec/threads.rb new file mode 100644 index 0000000000..b442a8f4fc --- /dev/null +++ b/lib/metasploit/framework/spec/threads.rb @@ -0,0 +1,5 @@ +module Metasploit::Framework::Spec::Threads + extend ActiveSupport::Autoload + + autoload :Suite +end \ No newline at end of file diff --git a/lib/metasploit/framework/spec/threads/suite.rb b/lib/metasploit/framework/spec/threads/suite.rb new file mode 100644 index 0000000000..acd17d7ed3 --- /dev/null +++ b/lib/metasploit/framework/spec/threads/suite.rb @@ -0,0 +1,35 @@ +module Metasploit::Framework::Spec::Threads::Suite + # + # CONSTANTS + # + + EXPECTED_THREAD_COUNT_BEFORE_SUITE = 1 + + # + # Module Methods + # + + # Configures `before(:suite)` and `after(:suite)` callback to detect thread leaks. + # + # @return [void] + def self.configure! + unless @configured + RSpec.configure do |config| + config.before(:suite) do + thread_count = Thread.list.count + + expect(thread_count).to( + (be <= EXPECTED_THREAD_COUNT_BEFORE_SUITE), + "#{thread_count} #{'thread'.pluralize(thread_count)} exist(s) when " \ + "only #{EXPECTED_THREAD_COUNT_BEFORE_SUITE} #{'thread'.pluralize(EXPECTED_THREAD_COUNT_BEFORE_SUITE)} " \ + "expected before suite runs" + ) + end + end + + @configured = true + end + + @configured + end +end \ No newline at end of file diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 1e1092fdec..d7e9db840b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -55,3 +55,5 @@ RSpec.configure do |config| # instead of true. config.use_transactional_fixtures = true end + +Metasploit::Framework::Spec::Threads::Suite.configure! From 097aa330e1aa6673bffdeee1c721e4f486390bc1 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Wed, 5 Nov 2014 15:34:35 -0600 Subject: [PATCH 02/34] Log caller for each Thread.new for `rake spec` MSP-11147 --- Rakefile | 1 + .../framework/spec/threads/logger.rb | 23 ++++++ .../framework/spec/threads/suite.rb | 82 +++++++++++++------ 3 files changed, 80 insertions(+), 26 deletions(-) create mode 100644 lib/metasploit/framework/spec/threads/logger.rb diff --git a/Rakefile b/Rakefile index c4be0fc98f..3acfdb97bf 100755 --- a/Rakefile +++ b/Rakefile @@ -10,4 +10,5 @@ require 'metasploit/framework/spec/untested_payloads' Metasploit::Framework::Require.optionally_active_record_railtie Metasploit::Framework::Application.load_tasks +Metasploit::Framework::Spec::Threads::Suite.define_task Metasploit::Framework::Spec::UntestedPayloads.define_task diff --git a/lib/metasploit/framework/spec/threads/logger.rb b/lib/metasploit/framework/spec/threads/logger.rb new file mode 100644 index 0000000000..ec67364a55 --- /dev/null +++ b/lib/metasploit/framework/spec/threads/logger.rb @@ -0,0 +1,23 @@ +require 'metasploit/framework/spec/threads/suite' + +original_thread_new = Thread.method(:new) + +# Patches `Thread.new` so that if logs `caller` so thread leaks can be traced +Thread.define_singleton_method(:new) { |*args, &block| + lines = ['BEGIN Thread.new caller'] + + caller.each do |frame| + lines << " #{frame}" + end + + lines << 'END Thread.new caller' + + Metasploit::Framework::Spec::Threads::Suite::LOG_PATHNAME.parent.mkpath + + Metasploit::Framework::Spec::Threads::Suite::LOG_PATHNAME.open('a') { |f| + # single puts so threads can't write in between each other. + f.puts lines.join("\n") + } + + original_thread_new.call(*args, &block) +} \ No newline at end of file diff --git a/lib/metasploit/framework/spec/threads/suite.rb b/lib/metasploit/framework/spec/threads/suite.rb index acd17d7ed3..d123e1554c 100644 --- a/lib/metasploit/framework/spec/threads/suite.rb +++ b/lib/metasploit/framework/spec/threads/suite.rb @@ -1,35 +1,65 @@ -module Metasploit::Framework::Spec::Threads::Suite - # - # CONSTANTS - # +require 'pathname' - EXPECTED_THREAD_COUNT_BEFORE_SUITE = 1 +# @note needs to use explicit nesting. so this file can be loaded directly without loading 'metasploit/framework' which +# allows for faster loading of rake tasks. +module Metasploit + module Framework + module Spec + module Threads + module Suite + # + # CONSTANTS + # - # - # Module Methods - # + # Number of allowed threads when threads are counted in `before(:suite)` + EXPECTED_THREAD_COUNT_BEFORE_SUITE = 1 + # `caller` for all Thread.new calls + LOG_PATHNAME = Pathname.new('log/metasploit/framework/spec/threads/suite.log') - # Configures `before(:suite)` and `after(:suite)` callback to detect thread leaks. - # - # @return [void] - def self.configure! - unless @configured - RSpec.configure do |config| - config.before(:suite) do - thread_count = Thread.list.count + # + # Module Methods + # - expect(thread_count).to( - (be <= EXPECTED_THREAD_COUNT_BEFORE_SUITE), - "#{thread_count} #{'thread'.pluralize(thread_count)} exist(s) when " \ - "only #{EXPECTED_THREAD_COUNT_BEFORE_SUITE} #{'thread'.pluralize(EXPECTED_THREAD_COUNT_BEFORE_SUITE)} " \ - "expected before suite runs" - ) + # Configures `before(:suite)` and `after(:suite)` callback to detect thread leaks. + # + # @return [void] + def self.configure! + unless @configured + RSpec.configure do |config| + config.before(:suite) do + thread_count = Thread.list.count + + # check with if first so that error message can be constructed lazily + if thread_count > EXPECTED_THREAD_COUNT_BEFORE_SUITE + log = LOG_PATHNAME.read() + + raise RuntimeError, + "#{thread_count} #{'thread'.pluralize(thread_count)} exist(s) when " \ + "only #{EXPECTED_THREAD_COUNT_BEFORE_SUITE} " \ + "#{'thread'.pluralize(EXPECTED_THREAD_COUNT_BEFORE_SUITE)} expected before suite runs:\n#{log}" + end + end + end + + @configured = true + end + + @configured + end + + def self.define_task + Rake::Task.define_task('metasploit:framework:spec:threads:suite') do + parent_pathname = Pathname.new(__FILE__).parent + threads_logger_pathname = parent_pathname.join('logger') + load_pathname = parent_pathname.parent.parent.parent.parent.expand_path + + ENV['RUBYOPT'] = "-I#{load_pathname} -r#{threads_logger_pathname} #{ENV['RUBYOPT']}" + end + + Rake::Task.define_task(spec: 'metasploit:framework:spec:threads:suite') + end end end - - @configured = true end - - @configured end end \ No newline at end of file From d66c98b34dc9b4405ef563ae54926412e7f93cde Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Wed, 5 Nov 2014 15:51:43 -0600 Subject: [PATCH 03/34] Remove prior log/metasploit/framework/spec/threads/suite.log MSP-11147 --- lib/metasploit/framework/spec/threads/suite.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/metasploit/framework/spec/threads/suite.rb b/lib/metasploit/framework/spec/threads/suite.rb index d123e1554c..a927244c84 100644 --- a/lib/metasploit/framework/spec/threads/suite.rb +++ b/lib/metasploit/framework/spec/threads/suite.rb @@ -49,6 +49,10 @@ module Metasploit def self.define_task Rake::Task.define_task('metasploit:framework:spec:threads:suite') do + if Metasploit::Framework::Spec::Threads::Suite::LOG_PATHNAME.exist? + Metasploit::Framework::Spec::Threads::Suite::LOG_PATHNAME.delete + end + parent_pathname = Pathname.new(__FILE__).parent threads_logger_pathname = parent_pathname.join('logger') load_pathname = parent_pathname.parent.parent.parent.parent.expand_path @@ -57,6 +61,8 @@ module Metasploit end Rake::Task.define_task(spec: 'metasploit:framework:spec:threads:suite') + + Rake::Task.define_task() end end end From c1f1222783567637cc5e6a4f3ad9190c22966b07 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Thu, 6 Nov 2014 09:07:11 -0600 Subject: [PATCH 04/34] Check that threads/suite.log exists before reading MSP-11147 Even with leaked threads, there may be no log if the suite is run without `rake spec`, such as when `rspec` is used directly to run a subset of specs. --- lib/metasploit/framework/spec/threads/suite.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/metasploit/framework/spec/threads/suite.rb b/lib/metasploit/framework/spec/threads/suite.rb index a927244c84..56ae59129d 100644 --- a/lib/metasploit/framework/spec/threads/suite.rb +++ b/lib/metasploit/framework/spec/threads/suite.rb @@ -31,7 +31,10 @@ module Metasploit # check with if first so that error message can be constructed lazily if thread_count > EXPECTED_THREAD_COUNT_BEFORE_SUITE - log = LOG_PATHNAME.read() + # LOG_PATHNAME may not exist if suite run without `rake spec` + if LOG_PATHNAME.exist? + log = LOG_PATHNAME.read() + end raise RuntimeError, "#{thread_count} #{'thread'.pluralize(thread_count)} exist(s) when " \ From 8d06189a19b040916fa9a3f6b481c580b01c3bc4 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Thu, 6 Nov 2014 09:10:04 -0600 Subject: [PATCH 05/34] Tell use to run with `rake spec` to see Thread.new caller MSP-11147 If the log isn't available, tell the user to rerun with `rake spec` instead of printing nothing after the `:\n`, which looks incomplete. --- lib/metasploit/framework/spec/threads/suite.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/metasploit/framework/spec/threads/suite.rb b/lib/metasploit/framework/spec/threads/suite.rb index 56ae59129d..f6c6b7ee89 100644 --- a/lib/metasploit/framework/spec/threads/suite.rb +++ b/lib/metasploit/framework/spec/threads/suite.rb @@ -34,6 +34,8 @@ module Metasploit # LOG_PATHNAME may not exist if suite run without `rake spec` if LOG_PATHNAME.exist? log = LOG_PATHNAME.read() + else + log "Run `rake spec` to log where Thread.new is called." end raise RuntimeError, From 8855e0731cc09f7087897a36901bd695296fbdd7 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Thu, 6 Nov 2014 09:11:12 -0600 Subject: [PATCH 06/34] Fix multiline string indentation MSP-11147 --- lib/metasploit/framework/spec/threads/suite.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/metasploit/framework/spec/threads/suite.rb b/lib/metasploit/framework/spec/threads/suite.rb index f6c6b7ee89..ec125116dc 100644 --- a/lib/metasploit/framework/spec/threads/suite.rb +++ b/lib/metasploit/framework/spec/threads/suite.rb @@ -40,8 +40,9 @@ module Metasploit raise RuntimeError, "#{thread_count} #{'thread'.pluralize(thread_count)} exist(s) when " \ - "only #{EXPECTED_THREAD_COUNT_BEFORE_SUITE} " \ - "#{'thread'.pluralize(EXPECTED_THREAD_COUNT_BEFORE_SUITE)} expected before suite runs:\n#{log}" + "only #{EXPECTED_THREAD_COUNT_BEFORE_SUITE} " \ + "#{'thread'.pluralize(EXPECTED_THREAD_COUNT_BEFORE_SUITE)} expected before suite runs:\n" \ + "#{log}" end end end From 8f635a1d76f8e63d5a0d75bb09c48d3fe139a16d Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Thu, 6 Nov 2014 09:11:31 -0600 Subject: [PATCH 07/34] Remove empty define_task MSP-11147 --- lib/metasploit/framework/spec/threads/suite.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/metasploit/framework/spec/threads/suite.rb b/lib/metasploit/framework/spec/threads/suite.rb index ec125116dc..8608523bf6 100644 --- a/lib/metasploit/framework/spec/threads/suite.rb +++ b/lib/metasploit/framework/spec/threads/suite.rb @@ -67,8 +67,6 @@ module Metasploit end Rake::Task.define_task(spec: 'metasploit:framework:spec:threads:suite') - - Rake::Task.define_task() end end end From 8416985c9d0569b2834780f392e292653447f8a7 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Thu, 6 Nov 2014 14:05:35 -0600 Subject: [PATCH 08/34] Give Threads UUIDs for spec run so caller can be correlated Have 'metasploit/framework/spec/threads/suite/logger' generate a UUID for each Thread. This UUID is printed on the "BEGIN Thread.new caller" line and is assigned as a thread-local variable, 'metasploit/framework/spec/threads/logger/uuid'. In `after(:suite)`, the log can be parsed to map the caller back to each UUID and then only the UUID of the still existing threads is used to look up the caller and print their stacktraces. This means only leaked threads callers will be printed. --- .../framework/spec/threads/logger.rb | 22 ++- .../framework/spec/threads/suite.rb | 136 +++++++++++++++++- 2 files changed, 151 insertions(+), 7 deletions(-) diff --git a/lib/metasploit/framework/spec/threads/logger.rb b/lib/metasploit/framework/spec/threads/logger.rb index ec67364a55..88fe58d946 100644 --- a/lib/metasploit/framework/spec/threads/logger.rb +++ b/lib/metasploit/framework/spec/threads/logger.rb @@ -1,10 +1,22 @@ +# +# Standard Library +# + +require 'securerandom' + +# +# Project +# + require 'metasploit/framework/spec/threads/suite' original_thread_new = Thread.method(:new) # Patches `Thread.new` so that if logs `caller` so thread leaks can be traced Thread.define_singleton_method(:new) { |*args, &block| - lines = ['BEGIN Thread.new caller'] + uuid = SecureRandom.uuid + # tag caller with uuid so that only leaked threads caller needs to be printed + lines = ["BEGIN Thread.new caller (#{uuid})"] caller.each do |frame| lines << " #{frame}" @@ -19,5 +31,11 @@ Thread.define_singleton_method(:new) { |*args, &block| f.puts lines.join("\n") } - original_thread_new.call(*args, &block) + options = {original_args: args, uuid: uuid} + + original_thread_new.call(options) { + # record uuid for thread-leak detection can used uuid to correlate log with this thread. + Thread.current[Metasploit::Framework::Spec::Threads::Suite::UUID_THREAD_LOCAL_VARIABLE] = options.fetch(:uuid) + block.call(*options.fetch(:original_args)) + } } \ No newline at end of file diff --git a/lib/metasploit/framework/spec/threads/suite.rb b/lib/metasploit/framework/spec/threads/suite.rb index 8608523bf6..132624847d 100644 --- a/lib/metasploit/framework/spec/threads/suite.rb +++ b/lib/metasploit/framework/spec/threads/suite.rb @@ -11,10 +11,14 @@ module Metasploit # CONSTANTS # - # Number of allowed threads when threads are counted in `before(:suite)` - EXPECTED_THREAD_COUNT_BEFORE_SUITE = 1 + # Number of allowed threads when threads are counted in `after(:suite)` or `before(:suite)` + EXPECTED_THREAD_COUNT_AROUND_SUITE = 1 # `caller` for all Thread.new calls LOG_PATHNAME = Pathname.new('log/metasploit/framework/spec/threads/suite.log') + # Regular expression for extracting the UUID out of {LOG_PATHNAME} for each Thread.new caller block + UUID_REGEXP = /BEGIN Thread.new caller \((?.*)\)/ + # Name of thread local variable that Thread UUID is stored + UUID_THREAD_LOCAL_VARIABLE = "metasploit/framework/spec/threads/logger/uuid" # # Module Methods @@ -30,7 +34,7 @@ module Metasploit thread_count = Thread.list.count # check with if first so that error message can be constructed lazily - if thread_count > EXPECTED_THREAD_COUNT_BEFORE_SUITE + if thread_count > EXPECTED_THREAD_COUNT_AROUND_SUITE # LOG_PATHNAME may not exist if suite run without `rake spec` if LOG_PATHNAME.exist? log = LOG_PATHNAME.read() @@ -40,10 +44,61 @@ module Metasploit raise RuntimeError, "#{thread_count} #{'thread'.pluralize(thread_count)} exist(s) when " \ - "only #{EXPECTED_THREAD_COUNT_BEFORE_SUITE} " \ - "#{'thread'.pluralize(EXPECTED_THREAD_COUNT_BEFORE_SUITE)} expected before suite runs:\n" \ + "only #{EXPECTED_THREAD_COUNT_AROUND_SUITE} " \ + "#{'thread'.pluralize(EXPECTED_THREAD_COUNT_AROUND_SUITE)} expected before suite runs:\n" \ "#{log}" end + + LOG_PATHNAME.parent.mkpath + + LOG_PATHNAME.open('a') do |f| + # separator so after(:suite) can differentiate between threads created before(:suite) and during the + # suites + f.puts 'before(:suite)' + end + end + + config.after(:suite) do + LOG_PATHNAME.parent.mkpath + + LOG_PATHNAME.open('a') do |f| + # separator so that a flip flop can be used when reading the file below. Also useful if it turns + # out any threads are being created after this callback, which could be the case if another + # after(:suite) accidentally created threads by creating an Msf::Simple::Framework instance. + f.puts 'after(:suite)' + end + + thread_list = Thread.list + + thread_uuids = thread_list.map { |thread| + thread[Metasploit::Framework::Spec::Threads::Suite::UUID_THREAD_LOCAL_VARIABLE] + }.compact + + thread_count = thread_list.count + + if thread_count > EXPECTED_THREAD_COUNT_AROUND_SUITE + error_lines = [] + + if LOG_PATHNAME.exist? + caller_by_thread_uuid = Metasploit::Framework::Spec::Threads::Suite.caller_by_thread_uuid + + thread_uuids.each do |thread_uuid| + caller = caller_by_thread_uuid[thread_uuid] + + error_lines << "Thread #{thread_uuid}\n" + + error_lines.concat(caller) + end + else + error_lines << "Run `rake spec` to log where Thread.new is called." + end + + raise RuntimeError, + "#{thread_count} #{'thread'.pluralize(thread_count)} exist(s) when only " \ + "#{EXPECTED_THREAD_COUNT_AROUND_SUITE} " \ + "#{'thread'.pluralize(EXPECTED_THREAD_COUNT_AROUND_SUITE)} expected after suite runs:\n" \ + "#{error_lines.join}" + end end end @@ -68,6 +123,77 @@ module Metasploit Rake::Task.define_task(spec: 'metasploit:framework:spec:threads:suite') end + + # @note Ensure {LOG_PATHNAME} exists before calling. + # + # Yields each line of {LOG_PATHNAME} that happened during the suite run. + # + # @yield [line] + # @yieldparam line [String] a line in the {LOG_PATHNAME} between `before(:suite)` and `after(:suite)` + # @yieldreturn [void] + def self.each_suite_line + in_suite = false + + LOG_PATHNAME.each_line do |line| + if in_suite + if line.start_with?('after(:suite)') + break + else + yield line + end + else + if line.start_with?('before(:suite)') + in_suite = true + end + end + end + end + + # @note Ensure {LOG_PATHNAME} exists before calling. + # + # Yield each line for each Thread UUID gathered during the suite run. + # + # @yield [uuid, line] + # @yieldparam uuid [String] the UUID of thread thread + # @yieldparam line [String] a line in the `caller` for the given `uuid` + # @yieldreturn [void] + def self.each_thread_line + in_thread_caller = false + uuid = nil + + each_suite_line do |line| + if in_thread_caller + if line.start_with?('END Thread.new caller') + in_thread_caller = false + next + else + yield uuid, line + end + else + match = line.match(UUID_REGEXP) + + if match + in_thread_caller = true + uuid = match[:uuid] + end + end + end + end + + # The `caller` for each Thread UUID. + # + # @return [Hash{String => Array}] + def self.caller_by_thread_uuid + lines_by_thread_uuid = Hash.new { |hash, uuid| + hash[uuid] = [] + } + + each_thread_line do |uuid, line| + lines_by_thread_uuid[uuid] << line + end + + lines_by_thread_uuid + end end end end From eede74be1ebeea51fce16e8cd562e74d061a1a11 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Tue, 11 Nov 2014 11:49:48 -0600 Subject: [PATCH 09/34] Extract 'Msf::Framework#threads cleaner' MSP-11147 Extract from 'Msf::Simple::Framework' the `after(:each)` that kills and joins threads from `framework.threads` into 'Msf::Framework#threads cleaner`. --- .../contexts/msf/framework/threads/cleaner.rb | 16 ++++++++++++++++ .../shared/contexts/msf/simple/framework.rb | 17 ++--------------- 2 files changed, 18 insertions(+), 15 deletions(-) create mode 100644 spec/support/shared/contexts/msf/framework/threads/cleaner.rb diff --git a/spec/support/shared/contexts/msf/framework/threads/cleaner.rb b/spec/support/shared/contexts/msf/framework/threads/cleaner.rb new file mode 100644 index 0000000000..2c018f59aa --- /dev/null +++ b/spec/support/shared/contexts/msf/framework/threads/cleaner.rb @@ -0,0 +1,16 @@ +shared_context 'Msf::Framework#threads cleaner' do + after(:each) do + # explicitly kill threads so that they don't exhaust connection pool + thread_manager = framework.threads + + thread_manager.each do |thread| + thread.kill + # ensure killed thread is cleaned up by VM + thread.join + end + + thread_manager.monitor.kill + # ensure killed thread is cleaned up by VM + thread_manager.monitor.join + end +end \ No newline at end of file diff --git a/spec/support/shared/contexts/msf/simple/framework.rb b/spec/support/shared/contexts/msf/simple/framework.rb index 91dd87dc68..5e4a9f57cc 100644 --- a/spec/support/shared/contexts/msf/simple/framework.rb +++ b/spec/support/shared/contexts/msf/simple/framework.rb @@ -3,6 +3,8 @@ require 'msf/base/simple/framework' require 'metasploit/framework' shared_context 'Msf::Simple::Framework' do + include_context 'Msf::Framework#threads cleaner' + let(:dummy_pathname) do Rails.root.join('spec', 'dummy') end @@ -26,19 +28,4 @@ shared_context 'Msf::Simple::Framework' do after(:each) do dummy_pathname.rmtree end - - after(:each) do - # explicitly kill threads so that they don't exhaust connection pool - thread_manager = framework.threads - - thread_manager.each do |thread| - thread.kill - # ensure killed thread is cleaned up by VM - thread.join - end - - thread_manager.monitor.kill - # ensure killed thread is cleaned up by VM - thread_manager.monitor.join - end end From cf0ecd036756e9deb88c05e8a8606a35cc49eb2a Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Tue, 11 Nov 2014 12:02:14 -0600 Subject: [PATCH 10/34] Fix thread leaks in TaskManager spec MSP-11147 --- spec/lib/msf/core/task_manager_spec.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/spec/lib/msf/core/task_manager_spec.rb b/spec/lib/msf/core/task_manager_spec.rb index 3271df1741..ba664c6ca3 100644 --- a/spec/lib/msf/core/task_manager_spec.rb +++ b/spec/lib/msf/core/task_manager_spec.rb @@ -4,10 +4,7 @@ require 'msf/core' require 'msf/core/task_manager' describe Msf::TaskManager do - - let(:framework) do - Msf::Framework.new - end + include_context 'Msf::Simple::Framework' let(:tm) do Msf::TaskManager.new(framework) From 36ab73b83aa3f3b7e056e88fd9f0bf5aef75afbf Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Tue, 11 Nov 2014 12:25:14 -0600 Subject: [PATCH 11/34] Extract Msfcli#framework MSP-11147 Expose Msfcli @framework as Msfcli#framework so that it can be set in tests. It also allows Msfcli#framework to lazily initialize and memoize to @framework. --- msfcli | 50 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/msfcli b/msfcli index e67530e713..78759116d7 100755 --- a/msfcli +++ b/msfcli @@ -16,10 +16,21 @@ require 'rex' class Msfcli + # + # Attributes + # + + # @attribute framework + # @return [Msf::Simple::Framework] + attr_writer :framework + + # + # initialize + # + def initialize(args) @args = {} @indent = ' ' - @framework = nil @args[:module_name] = args.shift # First argument should be the module name @args[:mode] = args.pop || 'h' # Last argument should be the mode @@ -31,6 +42,14 @@ class Msfcli end end + # + # Instance Methods + # + + def framework + @framework ||= Msf::Simple::Framework.create({'DeferModuleLoads'=>true}) + end + # # Returns a usage Rex table # @@ -283,7 +302,6 @@ class Msfcli # Initializes exploit/payload/encoder/nop modules. # def init_modules - @framework = Msf::Simple::Framework.create({'DeferModuleLoads'=>true}) $stdout.puts "[*] Initializing modules..." module_name = @args[:module_name] @@ -297,11 +315,11 @@ class Msfcli whitelist = generate_whitelist # Load up all the possible modules, this is where things get slow again - @framework.init_module_paths({:whitelist=>whitelist}) - if (@framework.modules.module_load_error_by_path.length > 0) + framework.init_module_paths({:whitelist=>whitelist}) + if (framework.modules.module_load_error_by_path.length > 0) print("Warning: The following modules could not be loaded!\n\n") - @framework.modules.module_load_error_by_path.each do |path, error| + framework.modules.module_load_error_by_path.each do |path, error| print("\t#{path}: #{error}\n\n") end @@ -310,16 +328,16 @@ class Msfcli # Determine what type of module it is if module_name =~ /exploit\/(.*)/ - modules[:module] = @framework.exploits.create($1) + modules[:module] = framework.exploits.create($1) elsif module_name =~ /auxiliary\/(.*)/ - modules[:module] = @framework.auxiliary.create($1) + modules[:module] = framework.auxiliary.create($1) elsif module_name =~ /post\/(.*)/ - modules[:module] = @framework.post.create($1) + modules[:module] = framework.post.create($1) else - modules[:module] = @framework.exploits.create(module_name) + modules[:module] = framework.exploits.create(module_name) if modules[:module].nil? # Try falling back on aux modules - modules[:module] = @framework.auxiliary.create(module_name) + modules[:module] = framework.auxiliary.create(module_name) end end @@ -342,7 +360,7 @@ class Msfcli # Create the payload to use if (modules[:module].datastore['PAYLOAD']) - modules[:payload] = @framework.payloads.create(modules[:module].datastore['PAYLOAD']) + modules[:payload] = framework.payloads.create(modules[:module].datastore['PAYLOAD']) if modules[:payload] modules[:payload].datastore.import_options_from_s(@args[:params].join('_|_'), '_|_') end @@ -350,7 +368,7 @@ class Msfcli # Create the encoder to use if modules[:module].datastore['ENCODER'] - modules[:encoder] = @framework.encoders.create(modules[:module].datastore['ENCODER']) + modules[:encoder] = framework.encoders.create(modules[:module].datastore['ENCODER']) if modules[:encoder] modules[:encoder].datastore.import_options_from_s(@args[:params].join('_|_'), '_|_') end @@ -358,7 +376,7 @@ class Msfcli # Create the NOP to use if modules[:module].datastore['NOP'] - modules[:nop] = @framework.nops.create(modules[:module].datastore['NOP']) + modules[:nop] = framework.nops.create(modules[:module].datastore['NOP']) if modules[:nop] modules[:nop].datastore.import_options_from_s(@args[:params].join('_|_'), '_|_') end @@ -454,7 +472,7 @@ class Msfcli Msf::Ui::Console::Driver::DefaultPrompt, Msf::Ui::Console::Driver::DefaultPromptChar, { - 'Framework' => @framework, + 'Framework' => framework, # When I use msfcli, chances are I want speed, so ASCII art fanciness # probably isn't much of a big deal for me. 'DisableBanner' => true @@ -474,7 +492,7 @@ class Msfcli con.run_single("exploit") # If we have sessions or jobs, keep running - if @framework.sessions.length > 0 or @framework.jobs.length > 0 + if framework.sessions.length > 0 or framework.jobs.length > 0 con.run else con.run_single("quit") @@ -558,7 +576,7 @@ class Msfcli end # Process special var/val pairs... - Msf::Ui::Common.process_cli_arguments(@framework, @args[:params]) + Msf::Ui::Common.process_cli_arguments(framework, @args[:params]) engage_mode(modules) $stdout.puts From 86379db65ce0ed30b02be49232f416616dc116b3 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Tue, 11 Nov 2014 12:32:22 -0600 Subject: [PATCH 12/34] Remove incorrect 'Class methods' context MSP-11147 --- spec/msfcli_spec.rb | 660 ++++++++++++++++++++++---------------------- 1 file changed, 329 insertions(+), 331 deletions(-) diff --git a/spec/msfcli_spec.rb b/spec/msfcli_spec.rb index 4f21ff5d49..c0b01956d3 100644 --- a/spec/msfcli_spec.rb +++ b/spec/msfcli_spec.rb @@ -22,145 +22,144 @@ describe Msfcli do fake.string end - context "Class methods" do - context ".initialize" do - it "should give me the correct module name in key :module_name after object initialization" do - args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" - cli = Msfcli.new(args.split(' ')) - cli.instance_variable_get(:@args)[:module_name].should eq('multi/handler') - end - - it "should give me the correct mode in key :mode after object initialization" do - args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" - cli = Msfcli.new(args.split(' ')) - cli.instance_variable_get(:@args)[:mode].should eq('E') - end - - it "should give me the correct module parameters after object initialization" do - args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" - cli = Msfcli.new(args.split(' ')) - cli.instance_variable_get(:@args)[:params].should eq(['payload=windows/meterpreter/reverse_tcp', 'lhost=127.0.0.1']) - end - - it "should give me an exploit name without the prefix 'exploit'" do - args = "exploit/windows/browser/ie_cbutton_uaf payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" - cli = Msfcli.new(args.split(' ')) - cli.instance_variable_get(:@args)[:module_name].should eq("windows/browser/ie_cbutton_uaf") - end - - it "should give me an exploit name without the prefix 'exploits'" do - args = "exploits/windows/browser/ie_cbutton_uaf payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" - cli = Msfcli.new(args.split(' ')) - cli.instance_variable_get(:@args)[:module_name].should eq("windows/browser/ie_cbutton_uaf") - end - - it "should set mode 's' (summary)" do - args = "multi/handler payload=windows/meterpreter/reverse_tcp s" - cli = Msfcli.new(args.split(' ')) - cli.instance_variable_get(:@args)[:mode].should eq('s') - end - - it "should set mode 'h' (help) as default" do - args = "multi/handler" - cli = Msfcli.new(args.split(' ')) - cli.instance_variable_get(:@args)[:mode].should eq('h') - end + context ".initialize" do + it "should give me the correct module name in key :module_name after object initialization" do + args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" + cli = Msfcli.new(args.split(' ')) + cli.instance_variable_get(:@args)[:module_name].should eq('multi/handler') end - context ".usage" do - it "should see a help menu" do - out = get_stdout { - cli = Msfcli.new([]) - cli.usage - } - out.should =~ /Usage/ - end + it "should give me the correct mode in key :mode after object initialization" do + args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" + cli = Msfcli.new(args.split(' ')) + cli.instance_variable_get(:@args)[:mode].should eq('E') end - # - # This one is slow because we're loading all modules - # - context ".dump_module_list" do - include_context 'Metasploit::Framework::Spec::Constants cleaner' - - it "it should dump a list of modules" do - tbl = '' - stdout = get_stdout { - cli = Msfcli.new([]) - tbl = cli.dump_module_list - } - tbl.should =~ /Exploits/ and stdout.should =~ /Please wait/ - end + it "should give me the correct module parameters after object initialization" do + args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" + cli = Msfcli.new(args.split(' ')) + cli.instance_variable_get(:@args)[:params].should eq(['payload=windows/meterpreter/reverse_tcp', 'lhost=127.0.0.1']) end - context ".guess_payload_name" do - cli = Msfcli.new([]) - - it "should contain matches nedded for windows/meterpreter/reverse_tcp" do - m = cli.guess_payload_name('windows/meterpreter/reverse_tcp') - m.should eq([/stages\/windows\/meterpreter/, /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/]) - end - - it "should contain matches needed for windows/shell/reverse_tcp" do - m = cli.guess_payload_name('windows/shell/reverse_tcp') - m.should eq([/stages\/windows\/shell/, /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/]) - end - - it "should contain matches needed for windows/shell_reverse_tcp" do - m = cli.guess_payload_name('windows/shell_reverse_tcp') - m.should eq([/stages\/windows\/shell/, /payloads\/(singles|stagers|stages)\/windows\/.*(shell_reverse_tcp)\.rb$/]) - end - - it "should contain matches needed for php/meterpreter_reverse_tcp" do - m = cli.guess_payload_name('php/meterpreter_reverse_tcp') - m.should eq([/stages\/php\/meterpreter/, /payloads\/(stagers|stages)\/php\/.*(meterpreter_reverse_tcp)\.rb$/]) - end - - it "should contain matches needed for linux/x86/meterpreter/reverse_tcp" do - m = cli.guess_payload_name('linux/x86/meterpreter/reverse_tcp') - m.should eq([/stages\/linux\/x86\/meterpreter/, /payloads\/(stagers|stages)\/linux\/x86\/.*(reverse_tcp)\.rb$/]) - end - - it "should contain matches needed for java/meterpreter/reverse_tcp" do - m = cli.guess_payload_name('java/meterpreter/reverse_tcp') - m.should eq([/stages\/java\/meterpreter/, /payloads\/(stagers|stages)\/java\/.*(reverse_tcp)\.rb$/]) - end - - it "should contain matches needed for cmd/unix/reverse" do - m = cli.guess_payload_name('cmd/unix/reverse') - m.should eq([/stages\/cmd\/shell/, /payloads\/(singles|stagers|stages)\/cmd\/.*(reverse)\.rb$/]) - end - - it "should contain matches needed for bsd/x86/shell_reverse_tcp" do - m = cli.guess_payload_name('bsd/x86/shell_reverse_tcp') - m.should eq([/stages\/bsd\/x86\/shell/, /payloads\/(singles|stagers|stages)\/bsd\/x86\/.*(shell_reverse_tcp)\.rb$/]) - end + it "should give me an exploit name without the prefix 'exploit'" do + args = "exploit/windows/browser/ie_cbutton_uaf payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" + cli = Msfcli.new(args.split(' ')) + cli.instance_variable_get(:@args)[:module_name].should eq("windows/browser/ie_cbutton_uaf") end - context ".guess_encoder_name" do - cli = Msfcli.new([]) - it "should contain a match for x86/shikata_ga_nai" do - encoder = 'x86/shikata_ga_nai' - m = cli.guess_encoder_name(encoder) - m.should eq([/encoders\/#{encoder}/]) - end + it "should give me an exploit name without the prefix 'exploits'" do + args = "exploits/windows/browser/ie_cbutton_uaf payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" + cli = Msfcli.new(args.split(' ')) + cli.instance_variable_get(:@args)[:module_name].should eq("windows/browser/ie_cbutton_uaf") end - context ".guess_nop_name" do - cli = Msfcli.new([]) - it "should contain a match for guess_nop_name" do - nop = 'x86/single_byte' - m = cli.guess_nop_name(nop) - m.should eq([/nops\/#{nop}/]) - end + it "should set mode 's' (summary)" do + args = "multi/handler payload=windows/meterpreter/reverse_tcp s" + cli = Msfcli.new(args.split(' ')) + cli.instance_variable_get(:@args)[:mode].should eq('s') end - context ".generate_whitelist" do - it "should generate a whitelist for linux/x86/shell/reverse_tcp with encoder x86/fnstenv_mov" do - args = "multi/handler payload=linux/x86/shell/reverse_tcp lhost=127.0.0.1 encoder=x86/fnstenv_mov E" - cli = Msfcli.new(args.split(' ')) - list = cli.generate_whitelist.map { |e| e.to_s } - answer = [ + it "should set mode 'h' (help) as default" do + args = "multi/handler" + cli = Msfcli.new(args.split(' ')) + cli.instance_variable_get(:@args)[:mode].should eq('h') + end + end + + context ".usage" do + it "should see a help menu" do + out = get_stdout { + cli = Msfcli.new([]) + cli.usage + } + out.should =~ /Usage/ + end + end + + # + # This one is slow because we're loading all modules + # + context ".dump_module_list" do + include_context 'Metasploit::Framework::Spec::Constants cleaner' + + it "it should dump a list of modules" do + tbl = '' + stdout = get_stdout { + cli = Msfcli.new([]) + tbl = cli.dump_module_list + } + tbl.should =~ /Exploits/ and stdout.should =~ /Please wait/ + end + end + + context ".guess_payload_name" do + cli = Msfcli.new([]) + + it "should contain matches nedded for windows/meterpreter/reverse_tcp" do + m = cli.guess_payload_name('windows/meterpreter/reverse_tcp') + m.should eq([/stages\/windows\/meterpreter/, /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/]) + end + + it "should contain matches needed for windows/shell/reverse_tcp" do + m = cli.guess_payload_name('windows/shell/reverse_tcp') + m.should eq([/stages\/windows\/shell/, /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/]) + end + + it "should contain matches needed for windows/shell_reverse_tcp" do + m = cli.guess_payload_name('windows/shell_reverse_tcp') + m.should eq([/stages\/windows\/shell/, /payloads\/(singles|stagers|stages)\/windows\/.*(shell_reverse_tcp)\.rb$/]) + end + + it "should contain matches needed for php/meterpreter_reverse_tcp" do + m = cli.guess_payload_name('php/meterpreter_reverse_tcp') + m.should eq([/stages\/php\/meterpreter/, /payloads\/(stagers|stages)\/php\/.*(meterpreter_reverse_tcp)\.rb$/]) + end + + it "should contain matches needed for linux/x86/meterpreter/reverse_tcp" do + m = cli.guess_payload_name('linux/x86/meterpreter/reverse_tcp') + m.should eq([/stages\/linux\/x86\/meterpreter/, /payloads\/(stagers|stages)\/linux\/x86\/.*(reverse_tcp)\.rb$/]) + end + + it "should contain matches needed for java/meterpreter/reverse_tcp" do + m = cli.guess_payload_name('java/meterpreter/reverse_tcp') + m.should eq([/stages\/java\/meterpreter/, /payloads\/(stagers|stages)\/java\/.*(reverse_tcp)\.rb$/]) + end + + it "should contain matches needed for cmd/unix/reverse" do + m = cli.guess_payload_name('cmd/unix/reverse') + m.should eq([/stages\/cmd\/shell/, /payloads\/(singles|stagers|stages)\/cmd\/.*(reverse)\.rb$/]) + end + + it "should contain matches needed for bsd/x86/shell_reverse_tcp" do + m = cli.guess_payload_name('bsd/x86/shell_reverse_tcp') + m.should eq([/stages\/bsd\/x86\/shell/, /payloads\/(singles|stagers|stages)\/bsd\/x86\/.*(shell_reverse_tcp)\.rb$/]) + end + end + + context ".guess_encoder_name" do + cli = Msfcli.new([]) + it "should contain a match for x86/shikata_ga_nai" do + encoder = 'x86/shikata_ga_nai' + m = cli.guess_encoder_name(encoder) + m.should eq([/encoders\/#{encoder}/]) + end + end + + context ".guess_nop_name" do + cli = Msfcli.new([]) + it "should contain a match for guess_nop_name" do + nop = 'x86/single_byte' + m = cli.guess_nop_name(nop) + m.should eq([/nops\/#{nop}/]) + end + end + + context ".generate_whitelist" do + it "should generate a whitelist for linux/x86/shell/reverse_tcp with encoder x86/fnstenv_mov" do + args = "multi/handler payload=linux/x86/shell/reverse_tcp lhost=127.0.0.1 encoder=x86/fnstenv_mov E" + cli = Msfcli.new(args.split(' ')) + list = cli.generate_whitelist.map { |e| e.to_s } + answer = [ /multi\/handler/, /stages\/linux\/x86\/shell/, /payloads\/(stagers|stages)\/linux\/x86\/.*(reverse_tcp)\.rb$/, @@ -168,16 +167,16 @@ describe Msfcli do /post\/.+/, /encoders\/generic\/*/, /nops\/.+/ - ].map { |e| e.to_s } + ].map { |e| e.to_s } - list.should eq(answer) - end + list.should eq(answer) + end - it "should generate a whitelist for windows/meterpreter/reverse_tcp with default options" do - args = 'multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E' - cli = Msfcli.new(args.split(' ')) - list = cli.generate_whitelist.map { |e| e.to_s } - answer = [ + it "should generate a whitelist for windows/meterpreter/reverse_tcp with default options" do + args = 'multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E' + cli = Msfcli.new(args.split(' ')) + list = cli.generate_whitelist.map { |e| e.to_s } + answer = [ /multi\/handler/, /stages\/windows\/meterpreter/, /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/, @@ -185,16 +184,16 @@ describe Msfcli do /encoders\/generic\/*/, /encoders\/.+/, /nops\/.+/ - ].map { |e| e.to_s } + ].map { |e| e.to_s } - list.should eq(answer) - end + list.should eq(answer) + end - it "should generate a whitelist for windows/meterpreter/reverse_tcp with options: encoder='' post='' nop=''" do - args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 encoder='' post='' nop='' E" - cli = Msfcli.new(args.split(' ')) - list = cli.generate_whitelist.map { |e| e.to_s } - answer = [ + it "should generate a whitelist for windows/meterpreter/reverse_tcp with options: encoder='' post='' nop=''" do + args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 encoder='' post='' nop='' E" + cli = Msfcli.new(args.split(' ')) + list = cli.generate_whitelist.map { |e| e.to_s } + answer = [ /multi\/handler/, /stages\/windows\/meterpreter/, /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/, @@ -202,221 +201,220 @@ describe Msfcli do /post\/''/, /nops\/''/, /encoders\/generic\/*/ - ].map { |e| e.to_s } + ].map { |e| e.to_s } - list.should eq(answer) - end + list.should eq(answer) + end - it "should generate a whitelist for windows/meterpreter/reverse_tcp with options: encoder= post= nop=" do - args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 encoder= post= nop= E" - cli = Msfcli.new(args.split(' ')) - list = cli.generate_whitelist.map { |e| e.to_s } - answer = [ + it "should generate a whitelist for windows/meterpreter/reverse_tcp with options: encoder= post= nop=" do + args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 encoder= post= nop= E" + cli = Msfcli.new(args.split(' ')) + list = cli.generate_whitelist.map { |e| e.to_s } + answer = [ /multi\/handler/, /stages\/windows\/meterpreter/, /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/, /encoders\/generic\/*/ - ].map { |e| e.to_s } + ].map { |e| e.to_s } - list.should eq(answer) - end + list.should eq(answer) + end + end + + context ".init_modules" do + include_context 'Metasploit::Framework::Spec::Constants cleaner' + + it "should inititalize an exploit module" do + args = 'exploit/windows/smb/psexec S' + m = '' + stdout = get_stdout { + cli = Msfcli.new(args.split(' ')) + m = cli.init_modules + } + m[:module].class.to_s.should start_with("Msf::Modules::Mod") end - context ".init_modules" do - include_context 'Metasploit::Framework::Spec::Constants cleaner' - - it "should inititalize an exploit module" do - args = 'exploit/windows/smb/psexec S' - m = '' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - } - m[:module].class.to_s.should start_with("Msf::Modules::Mod") - end - - it "should inititalize an auxiliary module" do - args = 'auxiliary/server/browser_autopwn S' - m = '' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - } - m[:module].class.to_s.should start_with("Msf::Modules::Mod") - end - - it "should inititalize a post module" do - args = 'post/windows/gather/credentials/gpp S' - m = '' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - } - m[:module].class.to_s.should start_with("Msf::Modules::Mod") - end - - it "should have multi/handler module initialized" do - args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" - m = '' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - } - - m[:module].class.to_s.should =~ /^Msf::Modules::/ - end - - it "should have my payload windows/meterpreter/reverse_tcp initialized" do - args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" - m = '' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - } - - m[:payload].class.to_s.should =~ / Date: Tue, 11 Nov 2014 13:11:05 -0600 Subject: [PATCH 13/34] Update Msfcli#initialize spec style MSP-11147 --- spec/msfcli_spec.rb | 122 ++++++++++++++++++++++++++++++++------------ 1 file changed, 88 insertions(+), 34 deletions(-) diff --git a/spec/msfcli_spec.rb b/spec/msfcli_spec.rb index c0b01956d3..c42cde55a9 100644 --- a/spec/msfcli_spec.rb +++ b/spec/msfcli_spec.rb @@ -8,6 +8,13 @@ require 'msf/base' describe Msfcli do + subject(:msfcli) { + described_class.new(args) + } + + let(:args) { + [] + } # Get stdout: # http://stackoverflow.com/questions/11349270/test-output-to-command-line-with-rspec @@ -23,46 +30,93 @@ describe Msfcli do end context ".initialize" do - it "should give me the correct module name in key :module_name after object initialization" do - args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" - cli = Msfcli.new(args.split(' ')) - cli.instance_variable_get(:@args)[:module_name].should eq('multi/handler') - end + context 'with module name' do + let(:args) { + [ + module_name, + *params + ] + } - it "should give me the correct mode in key :mode after object initialization" do - args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" - cli = Msfcli.new(args.split(' ')) - cli.instance_variable_get(:@args)[:mode].should eq('E') - end + let(:module_name) { + 'multi/handler' + } - it "should give me the correct module parameters after object initialization" do - args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" - cli = Msfcli.new(args.split(' ')) - cli.instance_variable_get(:@args)[:params].should eq(['payload=windows/meterpreter/reverse_tcp', 'lhost=127.0.0.1']) - end + let(:params) { + %w{payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1} + } - it "should give me an exploit name without the prefix 'exploit'" do - args = "exploit/windows/browser/ie_cbutton_uaf payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" - cli = Msfcli.new(args.split(' ')) - cli.instance_variable_get(:@args)[:module_name].should eq("windows/browser/ie_cbutton_uaf") - end + let(:parsed_args) { + msfcli.instance_variable_get(:@args) + } - it "should give me an exploit name without the prefix 'exploits'" do - args = "exploits/windows/browser/ie_cbutton_uaf payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" - cli = Msfcli.new(args.split(' ')) - cli.instance_variable_get(:@args)[:module_name].should eq("windows/browser/ie_cbutton_uaf") - end + context 'multi/handler' do + context 'with mode' do + let(:args) { + super() + [mode] + } - it "should set mode 's' (summary)" do - args = "multi/handler payload=windows/meterpreter/reverse_tcp s" - cli = Msfcli.new(args.split(' ')) - cli.instance_variable_get(:@args)[:mode].should eq('s') - end + context 'E' do + let(:mode) { + 'E' + } - it "should set mode 'h' (help) as default" do - args = "multi/handler" - cli = Msfcli.new(args.split(' ')) - cli.instance_variable_get(:@args)[:mode].should eq('h') + it 'parses module name into :module_name arg' do + expect(parsed_args[:module_name]).to eq(module_name) + end + + it 'parses mode into :mode arg' do + expect(parsed_args[:mode]).to eq(mode) + end + + it 'parses module parameters between module name and mode' do + expect(parsed_args[:params]).to eq(params) + end + end + + context 's' do + let(:mode) { + 's' + } + + it "parses mode as 's' (summary)" do + expect(parsed_args[:mode]).to eq(mode) + end + end + end + + context 'without mode' do + let(:args) { + [ + module_name + ] + } + + it "parses mode as 'h' (help) by default" do + expect(parsed_args[:mode]).to eq('h') + end + end + end + + context 'exploit/windows/browser/ie_cbutton_uaf' do + let(:module_name) { + 'exploit/windows/browser/ie_cbutton_uaf' + } + + it "strips 'exploit/' prefix for :module_name" do + expect(parsed_args[:module_name]).to eq('windows/browser/ie_cbutton_uaf') + end + end + + context 'exploit/windows/browser/ie_cbutton_uaf' do + let(:module_name) { + 'exploits/windows/browser/ie_cbutton_uaf' + } + + it "strips 'exploits/' prefix for :module_name" do + expect(parsed_args[:module_name]).to eq('windows/browser/ie_cbutton_uaf') + end + end end end From a6fed7798e01b6ae030fc2e2ec0cd34bc1ca5941 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Tue, 11 Nov 2014 13:11:40 -0600 Subject: [PATCH 14/34] Update Msfcli#usage spec style MSP-11147 --- spec/msfcli_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/msfcli_spec.rb b/spec/msfcli_spec.rb index c42cde55a9..5784397567 100644 --- a/spec/msfcli_spec.rb +++ b/spec/msfcli_spec.rb @@ -123,8 +123,7 @@ describe Msfcli do context ".usage" do it "should see a help menu" do out = get_stdout { - cli = Msfcli.new([]) - cli.usage + msfcli.usage } out.should =~ /Usage/ end From 5d6aec8bed27a4154b42d52fbe4fb068051e9291 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Tue, 11 Nov 2014 13:14:34 -0600 Subject: [PATCH 15/34] Fix context prefix MSP-11147 Instance methods should be prefixed with `#`, not `.`. --- spec/msfcli_spec.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/spec/msfcli_spec.rb b/spec/msfcli_spec.rb index 5784397567..1762921b7d 100644 --- a/spec/msfcli_spec.rb +++ b/spec/msfcli_spec.rb @@ -29,7 +29,7 @@ describe Msfcli do fake.string end - context ".initialize" do + context "#initialize" do context 'with module name' do let(:args) { [ @@ -120,7 +120,7 @@ describe Msfcli do end end - context ".usage" do + context "#usage" do it "should see a help menu" do out = get_stdout { msfcli.usage @@ -132,7 +132,7 @@ describe Msfcli do # # This one is slow because we're loading all modules # - context ".dump_module_list" do + context "#dump_module_list" do include_context 'Metasploit::Framework::Spec::Constants cleaner' it "it should dump a list of modules" do @@ -145,7 +145,7 @@ describe Msfcli do end end - context ".guess_payload_name" do + context "#guess_payload_name" do cli = Msfcli.new([]) it "should contain matches nedded for windows/meterpreter/reverse_tcp" do @@ -189,7 +189,7 @@ describe Msfcli do end end - context ".guess_encoder_name" do + context "#guess_encoder_name" do cli = Msfcli.new([]) it "should contain a match for x86/shikata_ga_nai" do encoder = 'x86/shikata_ga_nai' @@ -198,7 +198,7 @@ describe Msfcli do end end - context ".guess_nop_name" do + context "#guess_nop_name" do cli = Msfcli.new([]) it "should contain a match for guess_nop_name" do nop = 'x86/single_byte' @@ -207,7 +207,7 @@ describe Msfcli do end end - context ".generate_whitelist" do + context "#generate_whitelist" do it "should generate a whitelist for linux/x86/shell/reverse_tcp with encoder x86/fnstenv_mov" do args = "multi/handler payload=linux/x86/shell/reverse_tcp lhost=127.0.0.1 encoder=x86/fnstenv_mov E" cli = Msfcli.new(args.split(' ')) @@ -274,7 +274,7 @@ describe Msfcli do end end - context ".init_modules" do + context "#init_modules" do include_context 'Metasploit::Framework::Spec::Constants cleaner' it "should inititalize an exploit module" do @@ -352,7 +352,7 @@ describe Msfcli do end end - context ".engage_mode" do + context "#engage_mode" do include_context 'Metasploit::Framework::Spec::Constants cleaner' it "should show me the summary of module auxiliary/scanner/http/http_version" do From 56b53b0dcd4d2d399706c58664663c03e8100e05 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Tue, 11 Nov 2014 13:16:45 -0600 Subject: [PATCH 16/34] Remove redundant 'it' in text name MSP-11147 --- spec/msfcli_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/msfcli_spec.rb b/spec/msfcli_spec.rb index 1762921b7d..5423962084 100644 --- a/spec/msfcli_spec.rb +++ b/spec/msfcli_spec.rb @@ -135,7 +135,7 @@ describe Msfcli do context "#dump_module_list" do include_context 'Metasploit::Framework::Spec::Constants cleaner' - it "it should dump a list of modules" do + it "should dump a list of modules" do tbl = '' stdout = get_stdout { cli = Msfcli.new([]) From ebec5329dfbc3fbedf3e384a225d1d863e42f90a Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Tue, 11 Nov 2014 13:21:06 -0600 Subject: [PATCH 17/34] Update Msfclie#dump_module_list spec style MSP-11147 --- spec/msfcli_spec.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/msfcli_spec.rb b/spec/msfcli_spec.rb index 5423962084..341e917f50 100644 --- a/spec/msfcli_spec.rb +++ b/spec/msfcli_spec.rb @@ -135,13 +135,15 @@ describe Msfcli do context "#dump_module_list" do include_context 'Metasploit::Framework::Spec::Constants cleaner' - it "should dump a list of modules" do + it 'dumps a listof modules' do tbl = '' + stdout = get_stdout { - cli = Msfcli.new([]) - tbl = cli.dump_module_list + tbl = msfcli.dump_module_list } - tbl.should =~ /Exploits/ and stdout.should =~ /Please wait/ + + expect(tbl).to include 'Exploits' + expect(stdout).to include 'Please wait' end end From 1f1af70047bacd91e803ef694ed64a50dc104b43 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Tue, 11 Nov 2014 13:22:28 -0600 Subject: [PATCH 18/34] Update Msfcli#usage spec style MSP-11147 --- spec/msfcli_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/msfcli_spec.rb b/spec/msfcli_spec.rb index 341e917f50..2a868b1ef7 100644 --- a/spec/msfcli_spec.rb +++ b/spec/msfcli_spec.rb @@ -121,11 +121,12 @@ describe Msfcli do end context "#usage" do - it "should see a help menu" do + it "prints Usage" do out = get_stdout { msfcli.usage } - out.should =~ /Usage/ + + expect(out).to include('Usage') end end From d36da497d0da59db76f7f91ac58497be80ce41e9 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Tue, 11 Nov 2014 13:47:16 -0600 Subject: [PATCH 19/34] Update Msfcli#guess_payload_name spec style MSP-11147 --- spec/msfcli_spec.rb | 120 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 94 insertions(+), 26 deletions(-) diff --git a/spec/msfcli_spec.rb b/spec/msfcli_spec.rb index 2a868b1ef7..a7f63a9d52 100644 --- a/spec/msfcli_spec.rb +++ b/spec/msfcli_spec.rb @@ -149,46 +149,114 @@ describe Msfcli do end context "#guess_payload_name" do - cli = Msfcli.new([]) + subject(:guess_payload_name) { + msfcli.guess_payload_name(payload_reference_name) + } - it "should contain matches nedded for windows/meterpreter/reverse_tcp" do - m = cli.guess_payload_name('windows/meterpreter/reverse_tcp') - m.should eq([/stages\/windows\/meterpreter/, /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/]) + context 'with windows/meterpreter/reverse_tcp' do + let(:payload_reference_name) { + 'windows/meterpreter/reverse_tcp' + } + + it { + is_expected.to eq( + [ + /stages\/windows\/meterpreter/, + /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/ + ] + ) + } end - it "should contain matches needed for windows/shell/reverse_tcp" do - m = cli.guess_payload_name('windows/shell/reverse_tcp') - m.should eq([/stages\/windows\/shell/, /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/]) + context 'with windows/shell/reverse_tcp' do + let(:payload_reference_name) { + 'windows/shell/reverse_tcp' + } + + it { + is_expected.to eq( + [ + /stages\/windows\/shell/, + /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/ + ] + ) + } end - it "should contain matches needed for windows/shell_reverse_tcp" do - m = cli.guess_payload_name('windows/shell_reverse_tcp') - m.should eq([/stages\/windows\/shell/, /payloads\/(singles|stagers|stages)\/windows\/.*(shell_reverse_tcp)\.rb$/]) + context 'with php/meterpreter_reverse_tcp' do + let(:payload_reference_name) { + 'php/meterpreter_reverse_tcp' + } + + it { + is_expected.to eq( + [ + /stages\/php\/meterpreter/, + /payloads\/(stagers|stages)\/php\/.*(meterpreter_reverse_tcp)\.rb$/ + ] + ) + } end - it "should contain matches needed for php/meterpreter_reverse_tcp" do - m = cli.guess_payload_name('php/meterpreter_reverse_tcp') - m.should eq([/stages\/php\/meterpreter/, /payloads\/(stagers|stages)\/php\/.*(meterpreter_reverse_tcp)\.rb$/]) + context 'with linux/x86/meterpreter/reverse_tcp' do + let(:payload_reference_name) { + 'linux/x86/meterpreter/reverse_tcp' + } + + it { + is_expected.to eq( + [ + /stages\/linux\/x86\/meterpreter/, + /payloads\/(stagers|stages)\/linux\/x86\/.*(reverse_tcp)\.rb$/ + ] + ) + } end - it "should contain matches needed for linux/x86/meterpreter/reverse_tcp" do - m = cli.guess_payload_name('linux/x86/meterpreter/reverse_tcp') - m.should eq([/stages\/linux\/x86\/meterpreter/, /payloads\/(stagers|stages)\/linux\/x86\/.*(reverse_tcp)\.rb$/]) + context 'with java/meterpreter/reverse_tcp' do + let(:payload_reference_name) { + 'java/meterpreter/reverse_tcp' + } + + it { + is_expected.to eq( + [ + /stages\/java\/meterpreter/, + /payloads\/(stagers|stages)\/java\/.*(reverse_tcp)\.rb$/ + ] + ) + } end - it "should contain matches needed for java/meterpreter/reverse_tcp" do - m = cli.guess_payload_name('java/meterpreter/reverse_tcp') - m.should eq([/stages\/java\/meterpreter/, /payloads\/(stagers|stages)\/java\/.*(reverse_tcp)\.rb$/]) + context 'with cmd/unix/reverse' do + let(:payload_reference_name) { + 'cmd/unix/reverse' + } + + it { + is_expected.to eq( + [ + /stages\/cmd\/shell/, + /payloads\/(singles|stagers|stages)\/cmd\/.*(reverse)\.rb$/ + ] + ) + } end - it "should contain matches needed for cmd/unix/reverse" do - m = cli.guess_payload_name('cmd/unix/reverse') - m.should eq([/stages\/cmd\/shell/, /payloads\/(singles|stagers|stages)\/cmd\/.*(reverse)\.rb$/]) - end + context 'with bsd/x86/shell_reverse_tcp' do + let(:payload_reference_name) { + 'bsd/x86/shell_reverse_tcp' + } + + it { + is_expected.to eq( + [ + /stages\/bsd\/x86\/shell/, + /payloads\/(singles|stagers|stages)\/bsd\/x86\/.*(shell_reverse_tcp)\.rb$/ + ] + ) + } - it "should contain matches needed for bsd/x86/shell_reverse_tcp" do - m = cli.guess_payload_name('bsd/x86/shell_reverse_tcp') - m.should eq([/stages\/bsd\/x86\/shell/, /payloads\/(singles|stagers|stages)\/bsd\/x86\/.*(shell_reverse_tcp)\.rb$/]) end end From 577065f68d1cef387688b7e704e11c32515b2b84 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Tue, 11 Nov 2014 14:14:50 -0600 Subject: [PATCH 20/34] Update Msfcli#guess_encoder_name spec style MSP-11147 --- spec/msfcli_spec.rb | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/spec/msfcli_spec.rb b/spec/msfcli_spec.rb index a7f63a9d52..bc2444b080 100644 --- a/spec/msfcli_spec.rb +++ b/spec/msfcli_spec.rb @@ -261,11 +261,20 @@ describe Msfcli do end context "#guess_encoder_name" do - cli = Msfcli.new([]) - it "should contain a match for x86/shikata_ga_nai" do - encoder = 'x86/shikata_ga_nai' - m = cli.guess_encoder_name(encoder) - m.should eq([/encoders\/#{encoder}/]) + subject(:guess_encoder_name) { + msfcli.guess_encoder_name(encoder_reference_name) + } + + context 'with x86/shikata_ga_nai' do + let(:encoder_reference_name) { + 'x86/shikata_ga_nai' + } + + it { + is_expected.to eq( + [/encoders\/#{encoder_reference_name}/] + ) + } end end From 965607c7dce956ab9bdedb677fcf391244ac1680 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Tue, 11 Nov 2014 14:16:55 -0600 Subject: [PATCH 21/34] Update Msfcli#guess_nop_name spec style MSP-11147 --- spec/msfcli_spec.rb | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/spec/msfcli_spec.rb b/spec/msfcli_spec.rb index bc2444b080..491a71f8ab 100644 --- a/spec/msfcli_spec.rb +++ b/spec/msfcli_spec.rb @@ -277,13 +277,23 @@ describe Msfcli do } end end - + + context "#guess_nop_name" do - cli = Msfcli.new([]) - it "should contain a match for guess_nop_name" do - nop = 'x86/single_byte' - m = cli.guess_nop_name(nop) - m.should eq([/nops\/#{nop}/]) + subject(:guess_nop_name) { + msfcli.guess_nop_name(nop_reference_name) + } + + context 'with x86/shikata_ga_nai' do + let(:nop_reference_name) { + 'x86/single_byte' + } + + it { + is_expected.to eq( + [/nops\/#{nop_reference_name}/] + ) + } end end From bb07de3294c24fa976bbf3e55a7e289917e326d2 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Tue, 11 Nov 2014 14:49:48 -0600 Subject: [PATCH 22/34] Update Msfcli#generate_whitelist spec style MSP-11147 --- spec/msfcli_spec.rb | 223 +++++++++++++++++++++++++++++++++----------- 1 file changed, 167 insertions(+), 56 deletions(-) diff --git a/spec/msfcli_spec.rb b/spec/msfcli_spec.rb index 491a71f8ab..9dfd687d97 100644 --- a/spec/msfcli_spec.rb +++ b/spec/msfcli_spec.rb @@ -298,69 +298,180 @@ describe Msfcli do end context "#generate_whitelist" do - it "should generate a whitelist for linux/x86/shell/reverse_tcp with encoder x86/fnstenv_mov" do - args = "multi/handler payload=linux/x86/shell/reverse_tcp lhost=127.0.0.1 encoder=x86/fnstenv_mov E" - cli = Msfcli.new(args.split(' ')) - list = cli.generate_whitelist.map { |e| e.to_s } - answer = [ - /multi\/handler/, - /stages\/linux\/x86\/shell/, - /payloads\/(stagers|stages)\/linux\/x86\/.*(reverse_tcp)\.rb$/, - /encoders\/x86\/fnstenv_mov/, - /post\/.+/, - /encoders\/generic\/*/, - /nops\/.+/ - ].map { |e| e.to_s } + subject(:generate_whitelist) { + msfcli.generate_whitelist.map(&:to_s) + } - list.should eq(answer) - end + let(:args) { + [ + 'multi/handler', + "payload=#{payload_reference_name}", + 'lhost=127.0.0.1', + mode + ] + } - it "should generate a whitelist for windows/meterpreter/reverse_tcp with default options" do - args = 'multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E' - cli = Msfcli.new(args.split(' ')) - list = cli.generate_whitelist.map { |e| e.to_s } - answer = [ - /multi\/handler/, - /stages\/windows\/meterpreter/, - /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/, - /post\/.+/, - /encoders\/generic\/*/, - /encoders\/.+/, - /nops\/.+/ - ].map { |e| e.to_s } + let(:mode) { + 'E' + } - list.should eq(answer) - end + context 'with payload' do + context 'linux/x86/reverse_tcp' do + let(:payload_reference_name) { + 'linux/x86/reverse_tcp' + } - it "should generate a whitelist for windows/meterpreter/reverse_tcp with options: encoder='' post='' nop=''" do - args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 encoder='' post='' nop='' E" - cli = Msfcli.new(args.split(' ')) - list = cli.generate_whitelist.map { |e| e.to_s } - answer = [ - /multi\/handler/, - /stages\/windows\/meterpreter/, - /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/, - /encoders\/''/, - /post\/''/, - /nops\/''/, - /encoders\/generic\/*/ - ].map { |e| e.to_s } + context 'with encoder' do + let(:args) { + super().tap { |args| + args.insert(-2, "encoder=#{encoder_reference_name}") + } + } - list.should eq(answer) - end + context 'x86/fnstenv_mov' do + let(:encoder_reference_name) { + 'x86/fnstenv_mov' + } - it "should generate a whitelist for windows/meterpreter/reverse_tcp with options: encoder= post= nop=" do - args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 encoder= post= nop= E" - cli = Msfcli.new(args.split(' ')) - list = cli.generate_whitelist.map { |e| e.to_s } - answer = [ - /multi\/handler/, - /stages\/windows\/meterpreter/, - /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/, - /encoders\/generic\/*/ - ].map { |e| e.to_s } + it { + is_expected.to match_array( + [ + /multi\/handler/, + /stages\/linux\/x86\/shell/, + /payloads\/(singles|stagers|stages)\/linux\/x86\/.*(reverse_tcp)\.rb$/, + /encoders\/x86\/fnstenv_mov/, + /post\/.+/, + /encoders\/generic\/*/, + /nops\/.+/ + ].map(&:to_s) + ) + } + end + end + end - list.should eq(answer) + context 'windows/meterpreter/reverse_tcp' do + let(:payload_reference_name) { + 'windows/meterpreter/reverse_tcp' + } + + context 'with default options' do + it { + is_expected.to match_array( + [ + /multi\/handler/, + /stages\/windows\/meterpreter/, + /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/, + /post\/.+/, + /encoders\/generic\/*/, + /encoders\/.+/, + /nops\/.+/ + ].map(&:to_s) + ) + } + end + + context 'with encoder' do + let(:args) { + super().tap { |args| + args.insert(-2, "encoder=#{encoder_reference_name}") + } + } + + context "''" do + let(:encoder_reference_name) do + "''" + end + + context 'with post' do + let(:args) { + super().tap { |args| + args.insert(-2, "post=#{post_reference_name}") + } + } + + context "''" do + let(:post_reference_name) do + "''" + end + + context "with nop" do + let(:args) { + super().tap { |args| + args.insert(-2, "nop=#{nop_reference_name}") + } + } + + context "''" do + let(:nop_reference_name) { + "''" + } + + it { + is_expected.to match_array( + [ + /multi\/handler/, + /stages\/windows\/meterpreter/, + /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/, + /encoders\/''/, + /post\/''/, + /nops\/''/, + /encoders\/generic\/*/ + ].map(&:to_s) + ) + } + end + end + end + end + end + + context "" do + let(:encoder_reference_name) do + "" + end + + context 'with post' do + let(:args) { + super().tap { |args| + args.insert(-2, "post=#{post_reference_name}") + } + } + + context "" do + let(:post_reference_name) do + "" + end + + context "with nop" do + let(:args) { + super().tap { |args| + args.insert(-2, "nop=#{nop_reference_name}") + } + } + + context "" do + let(:nop_reference_name) { + "" + } + + it { + is_expected.to match_array( + [ + /multi\/handler/, + /stages\/windows\/meterpreter/, + /payloads\/(stagers|stages)\/windows\/.*(reverse_tcp)\.rb$/, + /encoders\/generic\/*/ + ].map(&:to_s) + ) + } + end + end + end + end + end + end + end end end From c0a3707c52b5ef3c3ea7289852c823b8b56f6895 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Tue, 11 Nov 2014 15:29:21 -0600 Subject: [PATCH 23/34] Update Msfcli#init_modules spec style MSP-11147 --- spec/msfcli_spec.rb | 190 ++++++++++++++++++++++++++++++-------------- 1 file changed, 131 insertions(+), 59 deletions(-) diff --git a/spec/msfcli_spec.rb b/spec/msfcli_spec.rb index 9dfd687d97..55af0979ac 100644 --- a/spec/msfcli_spec.rb +++ b/spec/msfcli_spec.rb @@ -478,78 +478,150 @@ describe Msfcli do context "#init_modules" do include_context 'Metasploit::Framework::Spec::Constants cleaner' - it "should inititalize an exploit module" do - args = 'exploit/windows/smb/psexec S' - m = '' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules + let(:args) { + [ + module_name, + mode + ] + } + + let(:mode) { + 'S' + } + + context 'with exploit/windows/smb/psexec' do + let(:module_name) { + 'exploit/windows/smb/psexec' } - m[:module].class.to_s.should start_with("Msf::Modules::Mod") + + it 'creates the module in :module' do + modules = {} + + Kernel.quietly { + modules = msfcli.init_modules + } + + expect(modules[:module]).to be_an Msf::Exploit + expect(modules[:module].fullname).to eq(module_name) + end end - it "should inititalize an auxiliary module" do - args = 'auxiliary/server/browser_autopwn S' - m = '' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules + context 'with auxiliary/server/browser_autopwn' do + let(:module_name) { + 'auxiliary/server/browser_autopwn' } - m[:module].class.to_s.should start_with("Msf::Modules::Mod") + + it 'creates the module in :module' do + modules = {} + + Kernel.quietly { + modules = msfcli.init_modules + } + + expect(modules[:module]).to be_an Msf::Auxiliary + expect(modules[:module].fullname).to eq(module_name) + end end - it "should inititalize a post module" do - args = 'post/windows/gather/credentials/gpp S' - m = '' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules + context 'with post/windows/gather/credentials/gpp' do + let(:module_name) { + 'post/windows/gather/credentials/gpp' } - m[:module].class.to_s.should start_with("Msf::Modules::Mod") + + it 'creates the module in :module' do + modules = {} + + Kernel.quietly { + modules = msfcli.init_modules + } + + expect(modules[:module]).to be_an Msf::Post + expect(modules[:module].fullname).to eq(module_name) + end + end + + context 'with multi/handler' do + let(:module_name) { + 'multi/handler' + } + + it 'creates the module in :module' do + modules = {} + + Kernel.quietly { + modules = msfcli.init_modules + } + + expect(modules[:module]).to be_an Msf::Exploit + expect(modules[:module].refname).to eq(module_name) + end + + context 'with payload' do + let(:args) { + super().tap { |args| + args.insert(-2, "payload=#{payload_reference_name}") + } + } + + context 'windows/meterpreter/reverse_tcp' do + let(:payload_reference_name) do + 'windows/meterpreter/reverse_tcp' + end + + it 'creates payload in :payload' do + modules = {} + + Kernel.quietly { + modules = msfcli.init_modules + } + + expect(modules[:payload]).to be_an Msf::Payload + expect(modules[:payload].refname).to eq(payload_reference_name) + end + end + end + + context 'with data store options' do + let(:args) { + super().tap { |args| + args.insert(-2, "#{data_store_key}=#{data_store_value}") + } + } + + let(:data_store_key) { + 'lhost' + } + + let(:data_store_value) { + '127.0.0.1' + } + + it 'sets data store on :module' do + modules = {} + + Kernel.quietly { + modules = msfcli.init_modules + } + + expect(modules[:module].datastore[data_store_key]).to eq(data_store_value) + end + end end - it "should have multi/handler module initialized" do - args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" - m = '' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules + context 'with invalid module name' do + let(:module_name) { + 'invalid/module/name' } - m[:module].class.to_s.should =~ /^Msf::Modules::/ - end + it 'returns empty modules Hash' do + modules = nil - it "should have my payload windows/meterpreter/reverse_tcp initialized" do - args = "multi/handler payload=windows/meterpreter/reverse_tcp lhost=127.0.0.1 E" - m = '' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - } + Kernel.quietly { + modules = msfcli.init_modules + } - m[:payload].class.to_s.should =~ / Date: Wed, 12 Nov 2014 09:08:36 -0600 Subject: [PATCH 24/34] Update Msfcli#engage_mode spec style MSP-11147 --- spec/msfcli_spec.rb | 236 ++++++++++++++++++++++++++------------------ 1 file changed, 138 insertions(+), 98 deletions(-) diff --git a/spec/msfcli_spec.rb b/spec/msfcli_spec.rb index 55af0979ac..5eb92da4c5 100644 --- a/spec/msfcli_spec.rb +++ b/spec/msfcli_spec.rb @@ -628,119 +628,159 @@ describe Msfcli do context "#engage_mode" do include_context 'Metasploit::Framework::Spec::Constants cleaner' - it "should show me the summary of module auxiliary/scanner/http/http_version" do - args = 'auxiliary/scanner/http/http_version s' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - cli.engage_mode(m) + subject(:engage_mode) { + msfcli.engage_mode(modules) + } + + let(:args) { + [ + module_name, + mode + ] + } + + let(:modules) { + msfcli.init_modules + } + + context 'with auxiliary/scanner/http/http_put' do + let(:module_name) { + 'auxiliary/scanner/http/http_put' } - stdout.should =~ /Module: auxiliary\/scanner\/http\/http_version/ + context 'with mode' do + context 'ac' do + let(:mode) { + 'ac' + } + + specify { + expect(get_stdout { engage_mode }).to match(/DELETE/) + } + end + end end - it "should show me the options of module auxiliary/scanner/http/http_version" do - args = 'auxiliary/scanner/http/http_version O' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - cli.engage_mode(m) + context 'with auxiliary/scanner/http/http_version' do + let(:module_name) { + 'auxiliary/scanner/http/http_version' } - stdout.should =~ /The target address range or CIDR identifier/ + context 'with mode' do + context 'A' do + let(:mode) { + 'A' + } + + specify { + expect(get_stdout { engage_mode }).to match(/UserAgent/) + } + end + + context 'I' do + let(:mode) { + 'I' + } + + specify { + expect(get_stdout { engage_mode }).to match(/Insert fake relative directories into the uri/) + } + end + + context 'O' do + let(:mode) { + 'O' + } + + specify { + expect(get_stdout { engage_mode }).to match(/The target address range or CIDR identifier/) + } + end + + context 'P' do + let(:mode) { + 'P' + } + + specify { + expect(get_stdout { engage_mode }).to match(/This type of module does not support payloads/) + } + end + + context 's' do + let(:mode) { + 's' + } + + specify { + expect(get_stdout { engage_mode }).to match %r{Module: auxiliary/scanner/http/http_version} + } + end + + context 't' do + let(:mode) { + 't' + } + + specify { + expect(get_stdout { engage_mode }).to match(/This type of module does not support targets/) + } + end + end end - it "should me the advanced options of module auxiliary/scanner/http/http_version" do - args = 'auxiliary/scanner/http/http_version A' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - cli.engage_mode(m) + context 'with windows/browser/ie_cbutton_uaf' do + let(:module_name) { + 'windows/browser/ie_cbutton_uaf' } - stdout.should =~ /UserAgent/ + context 'with mode' do + context 'ac' do + let(:mode) { + 'ac' + } + + specify { + expect(get_stdout { engage_mode }).to match(/This type of module does not support actions/) + } + end + + context 'P' do + let(:mode) { + 'P' + } + + specify { + expect(get_stdout { engage_mode }).to match(/windows\/meterpreter\/reverse_tcp/) + } + end + + context 'T' do + let(:mode) { + 'T' + } + + specify { + expect(get_stdout { engage_mode }).to match(/IE 8 on Windows 7/) + } + end + end end - it "should show me the IDS options of module auxiliary/scanner/http/http_version" do - args = 'auxiliary/scanner/http/http_version I' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - cli.engage_mode(m) + context 'with windows/smb/ms08_067_netapi' do + let(:module_name) { + 'windows/smb/ms08_067_netapi' } - stdout.should =~ /Insert fake relative directories into the uri/ - end - it "should show me the targets available for module windows/browser/ie_cbutton_uaf" do - args = "windows/browser/ie_cbutton_uaf T" - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - cli.engage_mode(m) - } - stdout.should =~ /IE 8 on Windows 7/ - end + context 'with mode C' do + let(:mode) { + 'C' + } - it "should show me the payloads available for module windows/browser/ie_cbutton_uaf" do - args = "windows/browser/ie_cbutton_uaf P" - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - cli.engage_mode(m) - } - stdout.should =~ /windows\/meterpreter\/reverse_tcp/ + specify { + expect(get_stdout { engage_mode }).to match(/#{Msf::Exploit::CheckCode::Unknown[1]}/) + } + end end - - it "should try to run the check function of an exploit" do - args = "windows/smb/ms08_067_netapi rhost=0.0.0.1 C" # Some BS IP so we can fail - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - cli.engage_mode(m) - } - stdout.should =~ /#{Msf::Exploit::CheckCode::Unknown[1]}/ - end - - it "should warn my auxiliary module isn't supported by mode 'p' (show payloads)" do - args = 'auxiliary/scanner/http/http_version p' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - cli.engage_mode(m) - } - stdout.should =~ /This type of module does not support payloads/ - end - - it "should warn my auxiliary module isn't supported by mode 't' (show targets)" do - args = 'auxiliary/scanner/http/http_version t' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - cli.engage_mode(m) - } - stdout.should =~ /This type of module does not support targets/ - end - - it "should warn my exploit module isn't supported by mode 'ac' (show actions)" do - args = 'windows/browser/ie_cbutton_uaf ac' - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - cli.engage_mode(m) - } - stdout.should =~ /This type of module does not support actions/ - end - - it "should show actions available for module auxiliary/scanner/http/http_put" do - args = "auxiliary/scanner/http/http_put ac" - stdout = get_stdout { - cli = Msfcli.new(args.split(' ')) - m = cli.init_modules - cli.engage_mode(m) - } - stdout.should =~ /DELETE/ - end - end - end From 8adc80fff10dd1b7785aac4a1992498ace1ece15 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Wed, 12 Nov 2014 09:16:37 -0600 Subject: [PATCH 25/34] Sort context entries MSP-11147 --- spec/msfcli_spec.rb | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/spec/msfcli_spec.rb b/spec/msfcli_spec.rb index 5eb92da4c5..d0b748fd7d 100644 --- a/spec/msfcli_spec.rb +++ b/spec/msfcli_spec.rb @@ -12,9 +12,9 @@ describe Msfcli do described_class.new(args) } - let(:args) { - [] - } + # + # methods + # # Get stdout: # http://stackoverflow.com/questions/11349270/test-output-to-command-line-with-rspec @@ -29,6 +29,14 @@ describe Msfcli do fake.string end + # + # lets + # + + let(:args) { + [] + } + context "#initialize" do context 'with module name' do let(:args) { From 61109d5567964a1430411ff5e3d6975fe5708d80 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Wed, 12 Nov 2014 12:13:53 -0600 Subject: [PATCH 26/34] Fix thread-leaks in msfcli spec MSP-11147 --- msfcli | 17 +++++++++++++++-- spec/msfcli_spec.rb | 21 +++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/msfcli b/msfcli index 78759116d7..92ba2932d3 100755 --- a/msfcli +++ b/msfcli @@ -22,7 +22,6 @@ class Msfcli # @attribute framework # @return [Msf::Simple::Framework] - attr_writer :framework # # initialize @@ -46,10 +45,24 @@ class Msfcli # Instance Methods # + # The framework to create and list modules. + # + # @return [Msf::Simple::Framework] def framework @framework ||= Msf::Simple::Framework.create({'DeferModuleLoads'=>true}) end + # Sets {#framework}. + # + # @raise [ArgumentError] if {#framework} already set + def framework=(framework) + if instance_variable_defined? :@framework + fail ArgumentError.new("framework already set") + end + + @framework = framework + end + # # Returns a usage Rex table # @@ -92,7 +105,7 @@ class Msfcli # msfcli will end up loading EVERYTHING to memory to show you a help # menu plus a list of modules available. Really expensive if you ask me. $stdout.puts "[*] Please wait while we load the module tree..." - framework = Msf::Simple::Framework.create + self.framework = Msf::Simple::Framework.create ext = '' tbl = Rex::Ui::Text::Table.new( diff --git a/spec/msfcli_spec.rb b/spec/msfcli_spec.rb index d0b748fd7d..6ca4045029 100644 --- a/spec/msfcli_spec.rb +++ b/spec/msfcli_spec.rb @@ -143,6 +143,11 @@ describe Msfcli do # context "#dump_module_list" do include_context 'Metasploit::Framework::Spec::Constants cleaner' + include_context 'Msf::Framework#threads cleaner' + + let(:framework) { + msfcli.framework + } it 'dumps a listof modules' do tbl = '' @@ -485,6 +490,7 @@ describe Msfcli do context "#init_modules" do include_context 'Metasploit::Framework::Spec::Constants cleaner' + include_context 'Msf::Framework#threads cleaner' let(:args) { [ @@ -493,6 +499,10 @@ describe Msfcli do ] } + let(:framework) { + msfcli.framework + } + let(:mode) { 'S' } @@ -635,6 +645,7 @@ describe Msfcli do context "#engage_mode" do include_context 'Metasploit::Framework::Spec::Constants cleaner' + include_context 'Msf::Framework#threads cleaner' subject(:engage_mode) { msfcli.engage_mode(modules) @@ -647,6 +658,10 @@ describe Msfcli do ] } + let(:framework) { + msfcli.framework + } + let(:modules) { msfcli.init_modules } @@ -776,6 +791,12 @@ describe Msfcli do end context 'with windows/smb/ms08_067_netapi' do + let(:args) { + super().tap { |args| + args.insert(-2, "RHOST=127.0.0.1") + } + } + let(:module_name) { 'windows/smb/ms08_067_netapi' } From 22cbc5ca024b46c4e002c8eae32b1d9e4cc55388 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Wed, 12 Nov 2014 12:18:08 -0600 Subject: [PATCH 27/34] Use named subject instead of subject MSP-11147 --- spec/lib/msf/core/framework_spec.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/lib/msf/core/framework_spec.rb b/spec/lib/msf/core/framework_spec.rb index 9c7094cced..2948ad836b 100644 --- a/spec/lib/msf/core/framework_spec.rb +++ b/spec/lib/msf/core/framework_spec.rb @@ -8,20 +8,20 @@ describe Msf::Framework do describe "#version" do CURRENT_VERSION = "4.10.1-dev" - subject do + subject(:framework) do described_class.new end it "should return the current version" do - subject.version.should == CURRENT_VERSION + framework.version.should == CURRENT_VERSION end it "should return the Version constant" do - described_class.const_get(:Version).should == subject.version + described_class.const_get(:Version).should == framework.version end it "should return the concatenation of Major.Minor.Point-Release" do - major,minor,point,release = subject.version.split(/[.-]/) + major,minor,point,release = framework.version.split(/[.-]/) major.to_i.should == described_class::Major minor.to_i.should == described_class::Minor point.to_i.should == described_class::Point @@ -30,7 +30,7 @@ describe Msf::Framework do skip "conform to SemVer 2.0 syntax: http://semver.org/" do it "should have constants that correspond to SemVer standards" do - major,minor,patch,label = subject.version.split(/[.-]/) + major,minor,patch,label = framework.version.split(/[.-]/) major.to_i.should == described_class::VERSION::MAJOR minor.to_i.should == described_class::VERSION::MINOR point.to_i.should == described_class::VERSION::POINT From 3ff87c89fe805cf16dc046adae1a7b4c9a95f68a Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Wed, 12 Nov 2014 12:20:23 -0600 Subject: [PATCH 28/34] Clean up Msf::Framework spec thread-leaks MSP-11147 --- spec/lib/msf/core/framework_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/lib/msf/core/framework_spec.rb b/spec/lib/msf/core/framework_spec.rb index 2948ad836b..d508576fb2 100644 --- a/spec/lib/msf/core/framework_spec.rb +++ b/spec/lib/msf/core/framework_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' require 'msf/core/framework' describe Msf::Framework do + include_context 'Msf::Framework#threads cleaner' describe "#version" do CURRENT_VERSION = "4.10.1-dev" From 44f78c21b2c7895a7e4a867aad7ccf294829296f Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Wed, 12 Nov 2014 12:27:33 -0600 Subject: [PATCH 29/34] Tag Msfcli spec as content MSP-11147 Tests currently use the real modules directory for test cases, so the spec should be tagged with :content because it has same performance issues as other content specs that can potentially load all the modules. --- spec/msfcli_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/msfcli_spec.rb b/spec/msfcli_spec.rb index 6ca4045029..c6dbd35978 100644 --- a/spec/msfcli_spec.rb +++ b/spec/msfcli_spec.rb @@ -7,7 +7,7 @@ require 'msf/ui' require 'msf/base' -describe Msfcli do +describe Msfcli, :content do subject(:msfcli) { described_class.new(args) } From 2fc6154ce971c772e26b6098baec2951b78bd3e9 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Wed, 12 Nov 2014 13:33:21 -0600 Subject: [PATCH 30/34] Update db/schema.rb MSP-11147 Must be missing on master too. --- db/schema.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index d2e6805207..83e99da75a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20140905031549) do +ActiveRecord::Schema.define(:version => 20140922170030) do create_table "api_keys", :force => true do |t| t.text "token" @@ -272,6 +272,7 @@ ActiveRecord::Schema.define(:version => 20140905031549) do t.string "username", :null => false t.datetime "created_at", :null => false t.datetime "updated_at", :null => false + t.string "type", :null => false end add_index "metasploit_credential_publics", ["username"], :name => "index_metasploit_credential_publics_on_username", :unique => true From 535f69b56d8c1f3179020058afd4580d5d2184b8 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Thu, 13 Nov 2014 09:19:07 -0600 Subject: [PATCH 31/34] Append to RUBYOPT for debugger compatibility MSP-11147 When using Rubymine's debugger, the tests would run and say there were no tests and no break points would be hit. It was determined that this was due the Rubymine's debugger injecting itself into RUBYOPTS and only working if it's first in RUBYOPT, which means that 'metasploit:framework:spec:threads:suite' must inject '-Ilib -rmetasploit/framework/spec/threads/logger' at the end of RUBOPT instead of the beginning. --- lib/metasploit/framework/spec/threads/suite.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/metasploit/framework/spec/threads/suite.rb b/lib/metasploit/framework/spec/threads/suite.rb index 132624847d..2a4b81ced3 100644 --- a/lib/metasploit/framework/spec/threads/suite.rb +++ b/lib/metasploit/framework/spec/threads/suite.rb @@ -118,7 +118,8 @@ module Metasploit threads_logger_pathname = parent_pathname.join('logger') load_pathname = parent_pathname.parent.parent.parent.parent.expand_path - ENV['RUBYOPT'] = "-I#{load_pathname} -r#{threads_logger_pathname} #{ENV['RUBYOPT']}" + # Must append to RUBYOPT or Rubymine debugger will not work + ENV['RUBYOPT'] = "#{ENV['RUBYOPT']} -I#{load_pathname} -r#{threads_logger_pathname}" end Rake::Task.define_task(spec: 'metasploit:framework:spec:threads:suite') From b17b263cc73f08abcc04b6b3ab9d202a9306256a Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Thu, 13 Nov 2014 09:49:08 -0600 Subject: [PATCH 32/34] Ignore debugger threads MSP-11147 When using the debugger, it adds a thread that should be allowed and not go towards the count. --- lib/metasploit/framework/spec/threads/suite.rb | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/metasploit/framework/spec/threads/suite.rb b/lib/metasploit/framework/spec/threads/suite.rb index 2a4b81ced3..99ef3a636a 100644 --- a/lib/metasploit/framework/spec/threads/suite.rb +++ b/lib/metasploit/framework/spec/threads/suite.rb @@ -31,7 +31,7 @@ module Metasploit unless @configured RSpec.configure do |config| config.before(:suite) do - thread_count = Thread.list.count + thread_count = Metasploit::Framework::Spec::Threads::Suite.non_debugger_thread_list.count # check with if first so that error message can be constructed lazily if thread_count > EXPECTED_THREAD_COUNT_AROUND_SUITE @@ -68,7 +68,7 @@ module Metasploit f.puts 'after(:suite)' end - thread_list = Thread.list + thread_list = Metasploit::Framework::Spec::Threads::Suite.non_debugger_thread_list thread_uuids = thread_list.map { |thread| thread[Metasploit::Framework::Spec::Threads::Suite::UUID_THREAD_LOCAL_VARIABLE] @@ -195,6 +195,15 @@ module Metasploit lines_by_thread_uuid end + + # @return + def self.non_debugger_thread_list + Thread.list.reject { |thread| + # don't do `is_a? Debugger::DebugThread` because it requires Debugger::DebugThread to be loaded, which it + # won't when not debugging. + thread.class.name == 'Debugger::DebugThread' + } + end end end end From ba836f2383fb437a2a5320d6cf85d24fe8da0f20 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Mon, 17 Nov 2014 09:17:44 -0600 Subject: [PATCH 33/34] Only calculate thread UUIDs if they are needed MSP-11147 Only calculate thread UUIDs if the thread count exceeds EXPECTED_THREAD_COUNT_AROUND_SUITE. --- lib/metasploit/framework/spec/threads/suite.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/metasploit/framework/spec/threads/suite.rb b/lib/metasploit/framework/spec/threads/suite.rb index 99ef3a636a..9df1aa1e06 100644 --- a/lib/metasploit/framework/spec/threads/suite.rb +++ b/lib/metasploit/framework/spec/threads/suite.rb @@ -69,11 +69,6 @@ module Metasploit end thread_list = Metasploit::Framework::Spec::Threads::Suite.non_debugger_thread_list - - thread_uuids = thread_list.map { |thread| - thread[Metasploit::Framework::Spec::Threads::Suite::UUID_THREAD_LOCAL_VARIABLE] - }.compact - thread_count = thread_list.count if thread_count > EXPECTED_THREAD_COUNT_AROUND_SUITE @@ -82,6 +77,10 @@ module Metasploit if LOG_PATHNAME.exist? caller_by_thread_uuid = Metasploit::Framework::Spec::Threads::Suite.caller_by_thread_uuid + thread_uuids = thread_list.map { |thread| + thread[Metasploit::Framework::Spec::Threads::Suite::UUID_THREAD_LOCAL_VARIABLE] + }.compact + thread_uuids.each do |thread_uuid| caller = caller_by_thread_uuid[thread_uuid] From e3869ee1ae6739a62dc2211c1546ff07fefdb740 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Mon, 17 Nov 2014 09:30:46 -0600 Subject: [PATCH 34/34] Include Thread status when printing leaked threads MSP-11147 Sometime travis-ci is showing leaked threads even when 'Msf::Framework#threads cleaner' is being used, so I'm adding the `Thread#status` to the data printed about the Thread to see if the sometimes leaked threads have an odd status. There's still a chance that there will be a race-condition between when I call Thread.list and I ask for each Thread's status that the VM could finish aborting a Thread so that status I print isn't the same as the one that caused the Thread to be returned in Thread.list. --- lib/metasploit/framework/spec/threads/suite.rb | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/metasploit/framework/spec/threads/suite.rb b/lib/metasploit/framework/spec/threads/suite.rb index 9df1aa1e06..c78791c579 100644 --- a/lib/metasploit/framework/spec/threads/suite.rb +++ b/lib/metasploit/framework/spec/threads/suite.rb @@ -77,14 +77,18 @@ module Metasploit if LOG_PATHNAME.exist? caller_by_thread_uuid = Metasploit::Framework::Spec::Threads::Suite.caller_by_thread_uuid - thread_uuids = thread_list.map { |thread| - thread[Metasploit::Framework::Spec::Threads::Suite::UUID_THREAD_LOCAL_VARIABLE] - }.compact + thread_list.each do |thread| + thread_uuid = thread[Metasploit::Framework::Spec::Threads::Suite::UUID_THREAD_LOCAL_VARIABLE] + + # unmanaged thread, such as the main VM thread + unless thread_uuid + next + end - thread_uuids.each do |thread_uuid| caller = caller_by_thread_uuid[thread_uuid] - error_lines << "Thread #{thread_uuid}\n" + error_lines << "Thread #{thread_uuid}'s status is #{thread.status.inspect} " \ + "and was started here:\n" error_lines.concat(caller) end