From 7796843f5b71a3e9725bf366e3722c4d34e3e315 Mon Sep 17 00:00:00 2001 From: Aaron Manaloto Date: Wed, 14 Aug 2024 16:03:24 +0800 Subject: [PATCH 1/3] Fix xpath and add stubbed mappings manager in specs --- CHANGELOG.md | 3 + lib/dradis/plugins/burp/html/importer.rb | 2 +- spec/html/importer_spec.rb | 69 ++++++++++ spec/spec_helper.rb | 9 ++ .../importer_spec.rb} | 118 ++++-------------- 5 files changed, 106 insertions(+), 95 deletions(-) create mode 100644 spec/html/importer_spec.rb rename spec/{burp_upload_spec.rb => xml/importer_spec.rb} (52%) diff --git a/CHANGELOG.md b/CHANGELOG.md index da2da7a..6028ca5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +v4.14.0 (Month 2024) + - Fix HTML importer associating issues in the wrong node + v4.13.0 (July 2024) - No changes diff --git a/lib/dradis/plugins/burp/html/importer.rb b/lib/dradis/plugins/burp/html/importer.rb index 8949597..292235a 100644 --- a/lib/dradis/plugins/burp/html/importer.rb +++ b/lib/dradis/plugins/burp/html/importer.rb @@ -90,7 +90,7 @@ def process_html_evidence(html_evidence, issue) evidence_id = html_evidence.attr('id').value logger.info { "Processing evidence #{evidence_id}" } - host_td = html_evidence.xpath("//td[starts-with(.,'Host:')]").first + host_td = html_evidence.xpath(".//td[starts-with(.,'Host:')]").first host_label = host_td.next_element.text.split('//').last host = content_service.create_node(label: host_label, type: :host) diff --git a/spec/html/importer_spec.rb b/spec/html/importer_spec.rb new file mode 100644 index 0000000..6a111e2 --- /dev/null +++ b/spec/html/importer_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' +require 'ostruct' + +describe 'Burp upload plugin' do + describe Dradis::Plugins::Burp::Html::Importer do + before(:each) do + # Stub mappings service + allow(Dradis::Plugins::MappingService).to receive(:new).and_return( + StubbedMappingService.new + ) + + # Init services + plugin = Dradis::Plugins::Burp::Html + + @content_service = Dradis::Plugins::ContentService::Base.new( + logger: Logger.new(STDOUT), + plugin: plugin + ) + + @importer = plugin::Importer.new( + content_service: @content_service, + ) + + # Stub dradis-plugins methods + # + # They return their argument hashes as objects mimicking + # Nodes, Issues, etc + allow(@content_service).to receive(:create_node) do |args| + obj = OpenStruct.new(args) + obj.define_singleton_method(:set_property) { |_, __| } + obj + end + allow(@content_service).to receive(:create_issue) do |args| + OpenStruct.new(args) + end + allow(@content_service).to receive(:create_evidence) do |args| + OpenStruct.new(args) + end + end + + it 'creates nodes, issues, and evidence as needed' do + # Host node + # + # create_node should be called once for each issue in the xml, + # but ContentService knows it's already created and NOOPs + expect(@content_service).to receive(:create_node) + .with(hash_including label: 'github.com/dradis/dradis-burp') + .exactly(1).times + + # # create_issue should be called once for each issue in the xml + expect(@content_service).to receive(:create_issue) do |args| + expect(args[:text]).to include("Strict transport security not enforced") + expect(args[:text]).to include('*application*', '@Wi-Fi@') + expect(args[:id]).to eq(16777984) + OpenStruct.new(args) + end.once + expect(@content_service).to receive(:create_evidence) do |args| + expect(args[:content]).to include('Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8') + expect(args[:content]).to include("http://1.1.1.1/dradis/sessions") + expect(args[:issue].text).to include("Strict transport security not enforced") + expect(args[:issue].text).to include('*application*', '@Wi-Fi@') + expect(args[:node].label).to eq('github.com/dradis/dradis-burp') + end.once + + # Run the import + @importer.import(file: 'spec/fixtures/files/burp.html') + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 475f25d..0026b00 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -7,3 +7,12 @@ RSpec.configure do |config| end + +class StubbedMappingService + def apply_mapping(args) + processor = Dradis::Plugins::Burp::FieldProcessor.new(data: args[:data]) + Dradis::Plugins::Burp::Mapping::SOURCE_FIELDS[args[:source].to_sym].map do |field| + processor.value(field: field) + end.join("\n") + end +end diff --git a/spec/burp_upload_spec.rb b/spec/xml/importer_spec.rb similarity index 52% rename from spec/burp_upload_spec.rb rename to spec/xml/importer_spec.rb index c3bf16d..c85fc56 100644 --- a/spec/burp_upload_spec.rb +++ b/spec/xml/importer_spec.rb @@ -2,7 +2,6 @@ require 'ostruct' describe 'Burp upload plugin' do - describe Burp::Xml::Issue do it 'handles invalid utf-8 bytes' do doc = Nokogiri::XML(File.read('spec/fixtures/files/invalid-utf-issue.xml')) @@ -15,10 +14,10 @@ describe Dradis::Plugins::Burp::Xml::Importer do before(:each) do - # Stub template service - templates_dir = File.expand_path('../../templates', __FILE__) - expect_any_instance_of(Dradis::Plugins::TemplateService) - .to receive(:default_templates_dir).and_return(templates_dir) + # Stub mappings service + allow(Dradis::Plugins::MappingService).to receive(:new).and_return( + StubbedMappingService.new + ) # Init services plugin = Dradis::Plugins::Burp::Xml @@ -50,7 +49,6 @@ end it 'creates nodes, issues, and evidence as needed' do - # Host node # # create_node should be called once for each issue in the xml, @@ -61,24 +59,24 @@ # create_issue should be called once for each issue in the xml expect(@content_service).to receive(:create_issue) do |args| - expect(args[:text]).to include("#[Title]#\nIssue 1") + expect(args[:text]).to include("Issue 1") expect(args[:id]).to eq(8781630) OpenStruct.new(args) end.once expect(@content_service).to receive(:create_evidence) do |args| expect(args[:content]).to include('Lorem ipsum dolor sit amet') - expect(args[:issue].text).to include("#[Title]#\nIssue 1") + expect(args[:issue].text).to include("Issue 1") expect(args[:node].label).to eq('10.0.0.1') end.once expect(@content_service).to receive(:create_issue) do |args| - expect(args[:text]).to include("#[Title]#\nIssue 2") + expect(args[:text]).to include("Issue 2") expect(args[:id]).to eq(8781631) OpenStruct.new(args) end.once expect(@content_service).to receive(:create_evidence) do |args| expect(args[:content]).to include('Lorem ipsum dolor sit amet') - expect(args[:issue].text).to include("#[Title]#\nIssue 2") + expect(args[:issue].text).to include("Issue 2") expect(args[:node].label).to eq('10.0.0.1') end.once @@ -86,24 +84,24 @@ # that it triggers process_extension_issues instead of process_burp_issues # and the plugin_id is not set to the Type (134217728) expect(@content_service).to receive(:create_issue) do |args| - expect(args[:text]).to include("#[Title]#\nIssue 3") + expect(args[:text]).to include("Issue 3") expect(args[:id]).to eq('Issue3') OpenStruct.new(args) end.once expect(@content_service).to receive(:create_evidence) do |args| expect(args[:content]).to include('Lorem ipsum dolor sit amet') - expect(args[:issue].text).to include("#[Title]#\nIssue 3") + expect(args[:issue].text).to include("Issue 3") expect(args[:node].label).to eq('10.0.0.1') end.once expect(@content_service).to receive(:create_issue) do |args| - expect(args[:text]).to include("#[Title]#\nIssue 4") + expect(args[:text]).to include("Issue 4") expect(args[:id]).to eq(8781633) OpenStruct.new(args) end.once expect(@content_service).to receive(:create_evidence) do |args| expect(args[:content]).to include('Lorem ipsum dolor sit amet') - expect(args[:issue].text).to include("#[Title]#\nIssue 4") + expect(args[:issue].text).to include("Issue 4") expect(args[:node].label).to eq('10.0.0.1') end.once @@ -112,38 +110,37 @@ end it 'returns the highest at the Issue level' do - expect(@content_service).to receive(:create_issue) do |args| expect(args[:id]).to eq(8781630) - expect(args[:text]).to include("#[Title]#\nIssue 1") - expect(args[:text]).to include("#[Severity]#\nInformation") + expect(args[:text]).to include("Issue 1") + expect(args[:text]).to include("Information") OpenStruct.new(args) end expect(@content_service).to receive(:create_evidence) do |args| - expect(args[:content]).to include("#[Severity]#\nInformation") - expect(args[:issue].text).to include("#[Title]#\nIssue 1") + expect(args[:content]).to include("Information") + expect(args[:issue].text).to include("Issue 1") expect(args[:node].label).to eq('10.0.0.1') end.once expect(@content_service).to receive(:create_evidence) do |args| - expect(args[:content]).to include("#[Severity]#\nHigh") - expect(args[:issue].text).to include("#[Title]#\nIssue 2") + expect(args[:content]).to include("High") + expect(args[:issue].text).to include("Issue 2") expect(args[:node].label).to eq('10.0.0.1') OpenStruct.new(args) end.once expect(@content_service).to receive(:create_evidence) do |args| - expect(args[:content]).to include("#[Severity]#\nMedium") - expect(args[:issue].text).to include("#[Title]#\nIssue 3") + expect(args[:content]).to include("Medium") + expect(args[:issue].text).to include("Issue 3") expect(args[:node].label).to eq('10.0.0.1') end.once expect(@content_service).to receive(:create_evidence) do |args| - expect(args[:content]).to include("#[Severity]#\nHigh") - expect(args[:issue].text).to include("#[Title]#\nIssue 4") + expect(args[:content]).to include("High") + expect(args[:issue].text).to include("Issue 4") expect(args[:node].label).to eq('10.0.0.1') end.once expect(@content_service).to receive(:create_evidence) do |args| - expect(args[:content]).to include("#[Severity]#\nLow") - expect(args[:issue].text).to include("#[Title]#\nIssue 5") + expect(args[:content]).to include("Low") + expect(args[:issue].text).to include("Issue 5") expect(args[:node].label).to eq('10.0.0.1') end.once @@ -151,71 +148,4 @@ @importer.import(file: 'spec/fixtures/files/burp_issue_severity.xml') end end - - describe Dradis::Plugins::Burp::Html::Importer do - before(:each) do - # Stub template service - templates_dir = File.expand_path('../../templates', __FILE__) - expect_any_instance_of(Dradis::Plugins::TemplateService) - .to receive(:default_templates_dir).and_return(templates_dir) - - # Init services - plugin = Dradis::Plugins::Burp::Html - - @content_service = Dradis::Plugins::ContentService::Base.new( - logger: Logger.new(STDOUT), - plugin: plugin - ) - - @importer = plugin::Importer.new( - content_service: @content_service, - ) - - # Stub dradis-plugins methods - # - # They return their argument hashes as objects mimicking - # Nodes, Issues, etc - allow(@content_service).to receive(:create_node) do |args| - obj = OpenStruct.new(args) - obj.define_singleton_method(:set_property) { |_, __| } - obj - end - allow(@content_service).to receive(:create_issue) do |args| - OpenStruct.new(args) - end - allow(@content_service).to receive(:create_evidence) do |args| - OpenStruct.new(args) - end - end - - it 'creates nodes, issues, and evidence as needed' do - - # Host node - # - # create_node should be called once for each issue in the xml, - # but ContentService knows it's already created and NOOPs - expect(@content_service).to receive(:create_node) - .with(hash_including label: 'github.com/dradis/dradis-burp') - .exactly(1).times - - # # create_issue should be called once for each issue in the xml - expect(@content_service).to receive(:create_issue) do |args| - expect(args[:text]).to include("#[Title]#\nStrict transport security not enforced") - expect(args[:text]).to include('*application*', '@Wi-Fi@') - expect(args[:id]).to eq(16777984) - OpenStruct.new(args) - end.once - expect(@content_service).to receive(:create_evidence) do |args| - expect(args[:content]).to include('Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8') - expect(args[:content]).to include("#[Location]#\nhttp://1.1.1.1/dradis/sessions") - expect(args[:issue].text).to include("#[Title]#\nStrict transport security not enforced") - expect(args[:issue].text).to include('*application*', '@Wi-Fi@') - expect(args[:node].label).to eq('github.com/dradis/dradis-burp') - end.once - - # Run the import - @importer.import(file: 'spec/fixtures/files/burp.html') - end - - end end From 2058eec0f571d865fec85499712579e5112c6807 Mon Sep 17 00:00:00 2001 From: Aaron Manaloto Date: Tue, 27 Aug 2024 13:07:33 +0800 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Caitlin Henry --- CHANGELOG.md | 2 +- lib/dradis/plugins/burp/html/importer.rb | 2 +- spec/html/importer_spec.rb | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6028ca5..5d5e4c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -v4.14.0 (Month 2024) +v4.X.X (Month 2024) - Fix HTML importer associating issues in the wrong node v4.13.0 (July 2024) diff --git a/lib/dradis/plugins/burp/html/importer.rb b/lib/dradis/plugins/burp/html/importer.rb index 292235a..ba6748f 100644 --- a/lib/dradis/plugins/burp/html/importer.rb +++ b/lib/dradis/plugins/burp/html/importer.rb @@ -90,7 +90,7 @@ def process_html_evidence(html_evidence, issue) evidence_id = html_evidence.attr('id').value logger.info { "Processing evidence #{evidence_id}" } - host_td = html_evidence.xpath(".//td[starts-with(.,'Host:')]").first + host_td = html_evidence.at_xpath(".//td[starts-with(.,'Host:')]") host_label = host_td.next_element.text.split('//').last host = content_service.create_node(label: host_label, type: :host) diff --git a/spec/html/importer_spec.rb b/spec/html/importer_spec.rb index 6a111e2..b25a2e7 100644 --- a/spec/html/importer_spec.rb +++ b/spec/html/importer_spec.rb @@ -54,6 +54,7 @@ expect(args[:id]).to eq(16777984) OpenStruct.new(args) end.once + expect(@content_service).to receive(:create_evidence) do |args| expect(args[:content]).to include('Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8') expect(args[:content]).to include("http://1.1.1.1/dradis/sessions") From ac9400744f4fc7953891f8a62bd2005408515a0e Mon Sep 17 00:00:00 2001 From: Aaron Manaloto Date: Tue, 27 Aug 2024 13:10:28 +0800 Subject: [PATCH 3/3] Move spec service under support/ --- spec/spec_helper.rb | 11 ++--------- spec/support/stubbed_mapping_service.rb | 8 ++++++++ 2 files changed, 10 insertions(+), 9 deletions(-) create mode 100644 spec/support/stubbed_mapping_service.rb diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0026b00..73cf55a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -3,16 +3,9 @@ require 'nokogiri' require 'combustion' +require 'support/stubbed_mapping_service' + Combustion.initialize! RSpec.configure do |config| end - -class StubbedMappingService - def apply_mapping(args) - processor = Dradis::Plugins::Burp::FieldProcessor.new(data: args[:data]) - Dradis::Plugins::Burp::Mapping::SOURCE_FIELDS[args[:source].to_sym].map do |field| - processor.value(field: field) - end.join("\n") - end -end diff --git a/spec/support/stubbed_mapping_service.rb b/spec/support/stubbed_mapping_service.rb new file mode 100644 index 0000000..4fa97bc --- /dev/null +++ b/spec/support/stubbed_mapping_service.rb @@ -0,0 +1,8 @@ +class StubbedMappingService + def apply_mapping(args) + processor = Dradis::Plugins::Burp::FieldProcessor.new(data: args[:data]) + Dradis::Plugins::Burp::Mapping::SOURCE_FIELDS[args[:source].to_sym].map do |field| + processor.value(field: field) + end.join("\n") + end +end