diff --git a/lib/msf/core/module_set.rb b/lib/msf/core/module_set.rb index 3befbfe787..fdb813140f 100644 --- a/lib/msf/core/module_set.rb +++ b/lib/msf/core/module_set.rb @@ -169,12 +169,14 @@ class Msf::ModuleSet < Hash end # @!attribute [rw] postpone_recalc - # Whether or not recalculations should be postponed. This is used from the context of the each_module_list - # handler in order to prevent the demand # loader from calling recalc for each module if it's possible that more - # than one module may be loaded. This field is not initialized until used. + # Whether or not recalculations should be postponed. This is used + # from the context of the {#each_module_list} handler in order to + # prevent the demand loader from calling recalc for each module if + # it's possible that more than one module may be loaded. This field + # is not initialized until used. # - # @return [true] if recalculate should not be called immediately - # @return [false] if recalculate should be called immediately + # @return [true] if {#recalculate} should not be called immediately + # @return [false] if {#recalculate} should be called immediately attr_accessor :postpone_recalculate # Dummy placeholder to recalculate aliases and other fun things. @@ -217,10 +219,10 @@ class Msf::ModuleSet < Hash # TODO this isn't terribly helpful since the refnames will always match, that's why they are ambiguous. wlog("The module #{mod.refname} is ambiguous with #{self[name].refname}.") - else - self[name] = mod end + self[name] = mod + mod end diff --git a/lib/msf/core/modules/loader/base.rb b/lib/msf/core/modules/loader/base.rb index 651e39e412..c96187d794 100644 --- a/lib/msf/core/modules/loader/base.rb +++ b/lib/msf/core/modules/loader/base.rb @@ -125,7 +125,7 @@ class Msf::Modules::Loader::Base return false end - loaded = namespace_module_transaction(type + "/" + module_reference_name, :reload => reload) { |namespace_module| + try_eval_module = lambda { |namespace_module| # set the parent_path so that the module can be reloaded with #load_module namespace_module.parent_path = parent_path @@ -173,29 +173,22 @@ class Msf::Modules::Loader::Base return false end - ilog("Loaded #{type} module #{module_reference_name} under #{parent_path}", 'core', LEV_2) + if reload + ilog("Reloading #{type} module #{module_reference_name}. Ambiguous module warnings are safe to ignore", 'core', LEV_2) + else + ilog("Loaded #{type} module #{module_reference_name} under #{parent_path}", 'core', LEV_2) + end module_manager.module_load_error_by_path.delete(module_path) true } + loaded = namespace_module_transaction(type + "/" + module_reference_name, :reload => reload, &try_eval_module) unless loaded return false end - if reload - # Delete the original copy of the module so that module_manager.on_load_module called from inside load_module does - # not trigger an ambiguous name warning, which would cause the reloaded module to not be stored in the - # ModuleManager. - module_manager.delete(module_reference_name) - - # Delete the original copy of the module in the type-specific module set stores the reloaded module and doesn't - # trigger an ambiguous name warning - module_set = module_manager.module_set(type) - module_set.delete(module_reference_name) - end - # Do some processing on the loaded module to get it into the right associations module_manager.on_module_load( metasploit_class, diff --git a/spec/lib/msf/core/modules/loader/base_spec.rb b/spec/lib/msf/core/modules/loader/base_spec.rb index efce31a1a0..0c4594c7e9 100644 --- a/spec/lib/msf/core/modules/loader/base_spec.rb +++ b/spec/lib/msf/core/modules/loader/base_spec.rb @@ -20,7 +20,7 @@ describe Msf::Modules::Loader::Base do let(:module_content) do <<-EOS - class Metasploit3 < Msf::Auxiliary + class Metasploit3 < Msf::Auxiliary # fully-qualified name is Msf::GoodRanking, so this will failing if lexical scope is not captured Rank = GoodRanking end @@ -106,10 +106,10 @@ describe Msf::Modules::Loader::Base do let(:namespace_module) do Object.module_eval( <<-EOS - module #{namespace_module_names[0]} + module #{namespace_module_names[0]} module #{namespace_module_names[1]} module #{namespace_module_names[2]} - #{described_class::NAMESPACE_MODULE_CONTENT} + #{described_class::NAMESPACE_MODULE_CONTENT} end end end @@ -255,18 +255,27 @@ describe Msf::Modules::Loader::Base do subject.stub(:module_path => module_path) end - it 'should return false if :force is false and the file has not been changed' do - module_manager.stub(:file_changed? => false) - - subject.load_module(parent_path, type, module_reference_name, :force => false).should be_false - end - it 'should call file_changed? with the module_path' do module_manager.should_receive(:file_changed?).with(module_path).and_return(false) subject.load_module(parent_path, type, module_reference_name, :force => false) end + context 'without file changed' do + before(:each) do + module_manager.stub(:file_changed? => false) + end + + it 'should return false if :force is false' do + subject.load_module(parent_path, type, module_reference_name, :force => false).should be_false + end + + it 'should not call #read_module_content' do + subject.should_not_receive(:read_module_content) + subject.load_module(parent_path, type, module_reference_name) + end + end + context 'with file changed' do let(:module_full_name) do File.join('auxiliary', module_reference_name) @@ -355,7 +364,7 @@ describe Msf::Modules::Loader::Base do end it 'should not attempt to make a new namespace_module' do - subject.should_not_receive(:namespace_module_transaction) + subject.should_not_receive(:namespace_module_transaction) subject.load_module(parent_path, type, module_reference_name).should be_false end end @@ -384,7 +393,7 @@ describe Msf::Modules::Loader::Base do let(:backtrace) do [ 'Backtrace Line 1', - 'Backtrace Line 2' + 'Backtrace Line 2' ] end @@ -424,9 +433,9 @@ describe Msf::Modules::Loader::Base do let(:version_compatibility_error) do Msf::Modules::VersionCompatibilityError.new( :module_path => module_path, - :module_reference_name => module_reference_name, - :minimum_api_version => infinity, - :minimum_core_version => infinity + :module_reference_name => module_reference_name, + :minimum_api_version => infinity, + :minimum_core_version => infinity ) end @@ -509,7 +518,7 @@ describe Msf::Modules::Loader::Base do end it 'should record the load error' do - subject.should_receive(:load_error).with(module_path, version_compatibility_error) + subject.should_receive(:load_error).with(module_path, version_compatibility_error) subject.load_module(parent_path, type, module_reference_name).should be_false end @@ -621,87 +630,6 @@ describe Msf::Modules::Loader::Base do subject.load_module(parent_path, type, module_reference_name).should be_true end - context 'with module_reference_name already in module_manager' do - let(:framework) do - framework = mock('Framework', :datastore => {}) - framework.stub_chain(:events, :on_module_load) - - framework - end - - let(:metasploit_class) do - @original_namespace_module::Metasploit3 - end - - let(:module_manager) do - Msf::ModuleManager.new(framework) - end - - before(:each) do - # remove the stub from before(:each) in context 'with version compatibility' - module_manager.unstub(:on_module_load) - - # remove the stubs from before(:each) in context 'with file changed' - module_manager.unstub(:delete) - module_manager.unstub(:module_set) - end - - it 'should not cause an ambiguous module_reference_name in the module_manager' do - module_manager[module_reference_name] = metasploit_class - - subject.load_module(parent_path, type, module_reference_name).should be_true - module_manager.send(:ambiguous_module_reference_name_set).should be_empty - end - - it 'should not cause an ambiguous module_reference_name in the type module_set' do - module_set = module_manager.module_set(type) - module_set[module_reference_name] = metasploit_class - - subject.load_module(parent_path, type, module_reference_name).should be_true - module_set.send(:ambiguous_module_reference_name_set).should be_empty - end - - context 'without file changed' do - before(:each) do - module_manager.stub(:file_changed => false) - end - - context 'with :force => true' do - it 'should not cause an ambiguous module_reference_name in the module_manager' do - module_manager[module_reference_name] = metasploit_class - - subject.load_module(parent_path, type, module_reference_name, :force => true).should be_true - module_manager.send(:ambiguous_module_reference_name_set).should be_empty - end - - it 'should not cause an ambiguous module_reference_name in the type module_set' do - module_set = module_manager.module_set(type) - module_set[module_reference_name] = metasploit_class - - subject.load_module(parent_path, type, module_reference_name, :force => true).should be_true - module_set.send(:ambiguous_module_reference_name_set).should be_empty - end - end - - context 'with :reload => true' do - it 'should not cause an ambiguous module_reference_name in the module_manager' do - module_manager[module_reference_name] = metasploit_class - - subject.load_module(parent_path, type, module_reference_name, :reload => true).should be_true - module_manager.send(:ambiguous_module_reference_name_set).should be_empty - end - - it 'should not cause an ambiguous module_reference_name in the type module_set' do - module_set = module_manager.module_set(type) - module_set[module_reference_name] = metasploit_class - - subject.load_module(parent_path, type, module_reference_name, :reload => true).should be_true - module_set.send(:ambiguous_module_reference_name_set).should be_empty - end - end - end - end - it 'should call module_manager.on_module_load' do module_manager.should_receive(:on_module_load) subject.load_module(parent_path, type, module_reference_name).should be_true @@ -728,9 +656,9 @@ describe Msf::Modules::Loader::Base do count_by_type.has_key?(type).should be_false subject.load_module( parent_path, - type, - module_reference_name, - :count_by_type => count_by_type + type, + module_reference_name, + :count_by_type => count_by_type ).should be_true count_by_type[type].should == 1 end @@ -743,9 +671,9 @@ describe Msf::Modules::Loader::Base do subject.load_module( parent_path, - type, - module_reference_name, - :count_by_type => count_by_type + type, + module_reference_name, + :count_by_type => count_by_type ).should be_true incremented_count = original_count + 1 @@ -763,8 +691,8 @@ describe Msf::Modules::Loader::Base do let(:namespace_module_names) do [ 'Msf', - 'Modules', - relative_name + 'Modules', + relative_name ] end @@ -794,8 +722,8 @@ describe Msf::Modules::Loader::Base do "end\n" \ "end\n" \ "end", - anything, - anything + anything, + anything ) namespace_module = mock('Namespace Module') @@ -810,8 +738,8 @@ describe Msf::Modules::Loader::Base do :module_eval ).with( anything, - described_class_pathname.to_s, - anything + described_class_pathname.to_s, + anything ) namespace_module = mock('Namespace Module') @@ -826,8 +754,8 @@ describe Msf::Modules::Loader::Base do :module_eval ).with( anything, - anything, - described_class::NAMESPACE_MODULE_LINE - namespace_module_names.length + anything, + described_class::NAMESPACE_MODULE_LINE - namespace_module_names.length ) namespace_module = mock('Namespace Module') @@ -861,7 +789,7 @@ describe Msf::Modules::Loader::Base do 'Mod0' end - before(:each) do + before(:each) do # copy to local variable so it is accessible in instance_eval relative_name = self.relative_name @@ -870,7 +798,7 @@ describe Msf::Modules::Loader::Base do remove_const relative_name end end - end + end it 'should return nil if the module is not defined' do Msf::Modules.const_defined?(relative_name).should be_false @@ -1019,7 +947,7 @@ describe Msf::Modules::Loader::Base do end it 'should remove the pre-existing namespace module' do - Msf::Modules.should_receive(:remove_const).with(relative_name) + Msf::Modules.should_receive(:remove_const).with(relative_name) subject.send(:namespace_module_transaction, module_full_name) do |namespace_module| true @@ -1129,13 +1057,13 @@ describe Msf::Modules::Loader::Base do end it 'should create a new namespace module' do - expect { - Msf::Modules.const_get(relative_name) - }.to raise_error(NameError) + expect { + Msf::Modules.const_get(relative_name) + }.to raise_error(NameError) - subject.send(:namespace_module_transaction, module_full_name) do |namespace_module| - Msf::Modules.const_get(relative_name).should == namespace_module - end + subject.send(:namespace_module_transaction, module_full_name) do |namespace_module| + Msf::Modules.const_get(relative_name).should == namespace_module + end end context 'with an Exception from the block' do @@ -1200,7 +1128,7 @@ describe Msf::Modules::Loader::Base do subject.send(:namespace_module_transaction, module_full_name) do |namespace_module| Msf::Modules.const_defined?(relative_name).should be_true - created_namespace_module = namespace_module + created_namespace_module = namespace_module true end @@ -1344,7 +1272,7 @@ describe Msf::Modules::Loader::Base do subject.send(:restore_namespace_module, parent_module, relative_name, @original_namespace_module) - parent_module.const_defined?(relative_name).should be_true + parent_module.const_defined?(relative_name).should be_true parent_module.const_get(relative_name).should == @original_namespace_module end end @@ -1353,10 +1281,10 @@ describe Msf::Modules::Loader::Base do context '#typed_path' do it 'should delegate to the class method' do - type = Msf::MODULE_EXPLOIT + type = Msf::MODULE_EXPLOIT - described_class.should_receive(:typed_path).with(type, module_reference_name) - subject.send(:typed_path, type, module_reference_name) + described_class.should_receive(:typed_path).with(type, module_reference_name) + subject.send(:typed_path, type, module_reference_name) end end @@ -1405,4 +1333,4 @@ describe Msf::Modules::Loader::Base do end end end -end \ No newline at end of file +end diff --git a/spec/support/shared/examples/typed_path.rb b/spec/support/shared/examples/typed_path.rb index d393fdb06b..a6e9b4a274 100644 --- a/spec/support/shared/examples/typed_path.rb +++ b/spec/support/shared/examples/typed_path.rb @@ -1,4 +1,5 @@ -shared_examples_for 'typed_path' do |map={}| +shared_examples_for 'typed_path' do |map| + map ||= {} if map.length < 1 raise ArgumentError, "type_path shared example requires a hash mapping the type constant name to the directory name: " \ @@ -24,4 +25,4 @@ shared_examples_for 'typed_path' do |map={}| first_directory.should == directory end end -end \ No newline at end of file +end