Commit 47e257cc authored by Alan (Maciej) Paruszewski's avatar Alan (Maciej) Paruszewski Committed by Ash McKenzie

Generate finding name from report data if message is missing

This change adds generating name from report data if message is missing.
Now we will use name or combination of identifier and location to
generate finding name.
parent c4a7dac3
---
title: Generate finding name from report data if message is missing
merge_request: 48279
author:
type: fixed
...@@ -56,13 +56,15 @@ module Gitlab ...@@ -56,13 +56,15 @@ module Gitlab
def create_vulnerability(report, data, version) def create_vulnerability(report, data, version)
identifiers = create_identifiers(report, data['identifiers']) identifiers = create_identifiers(report, data['identifiers'])
links = create_links(report, data['links']) links = create_links(report, data['links'])
location = create_location(data['location'] || {})
report.add_finding( report.add_finding(
::Gitlab::Ci::Reports::Security::Finding.new( ::Gitlab::Ci::Reports::Security::Finding.new(
uuid: SecureRandom.uuid, uuid: SecureRandom.uuid,
report_type: report.type, report_type: report.type,
name: data['message'], name: finding_name(data, identifiers, location),
compare_key: data['cve'] || '', compare_key: data['cve'] || '',
location: create_location(data['location'] || {}), location: location,
severity: parse_severity_level(data['severity']&.downcase), severity: parse_severity_level(data['severity']&.downcase),
confidence: parse_confidence_level(data['confidence']&.downcase), confidence: parse_confidence_level(data['confidence']&.downcase),
scanner: create_scanner(report, data['scanner']), scanner: create_scanner(report, data['scanner']),
...@@ -139,6 +141,16 @@ module Gitlab ...@@ -139,6 +141,16 @@ module Gitlab
def create_location(location_data) def create_location(location_data)
raise NotImplementedError raise NotImplementedError
end end
private
def finding_name(data, identifiers, location)
return data['message'] if data['message'].present?
return data['name'] if data['name'].present?
identifier = identifiers.find(&:cve?) || identifiers.find(&:cwe?) || identifiers.first
"#{identifier.name} in #{location&.fingerprint_path}"
end
end end
end end
end end
......
# frozen_string_literal: true
module Gitlab
module Ci
module Reports
module Security
module Concerns
module FingerprintPathFromFile
extend ActiveSupport::Concern
def fingerprint_path
File.basename(file_path.to_s)
end
end
end
end
end
end
end
...@@ -41,6 +41,14 @@ module Gitlab ...@@ -41,6 +41,14 @@ module Gitlab
other.external_id == external_id other.external_id == external_id
end end
def cve?
external_type.to_s.casecmp('cve') == 0
end
def cwe?
external_type.to_s.casecmp('cwe') == 0
end
private private
def generate_fingerprint def generate_fingerprint
......
...@@ -24,6 +24,10 @@ module Gitlab ...@@ -24,6 +24,10 @@ module Gitlab
super super
end end
def fingerprint_path
fingerprint_data
end
private private
def fingerprint_data def fingerprint_data
......
...@@ -18,6 +18,8 @@ module Gitlab ...@@ -18,6 +18,8 @@ module Gitlab
@path = path @path = path
end end
alias_method :fingerprint_path, :path
private private
def fingerprint_data def fingerprint_data
......
...@@ -6,6 +6,8 @@ module Gitlab ...@@ -6,6 +6,8 @@ module Gitlab
module Security module Security
module Locations module Locations
class DependencyScanning < Base class DependencyScanning < Base
include Security::Concerns::FingerprintPathFromFile
attr_reader :file_path attr_reader :file_path
attr_reader :package_name attr_reader :package_name
attr_reader :package_version attr_reader :package_version
......
...@@ -6,6 +6,8 @@ module Gitlab ...@@ -6,6 +6,8 @@ module Gitlab
module Security module Security
module Locations module Locations
class Sast < Base class Sast < Base
include Security::Concerns::FingerprintPathFromFile
attr_reader :class_name attr_reader :class_name
attr_reader :end_line attr_reader :end_line
attr_reader :file_path attr_reader :file_path
......
...@@ -6,6 +6,8 @@ module Gitlab ...@@ -6,6 +6,8 @@ module Gitlab
module Security module Security
module Locations module Locations
class SecretDetection < Base class SecretDetection < Base
include Security::Concerns::FingerprintPathFromFile
attr_reader :class_name attr_reader :class_name
attr_reader :end_line attr_reader :end_line
attr_reader :file_path attr_reader :file_path
......
...@@ -321,6 +321,16 @@ FactoryBot.define do ...@@ -321,6 +321,16 @@ FactoryBot.define do
end end
end end
trait :common_security_report_with_blank_names do
file_format { :raw }
file_type { :dependency_scanning }
after(:build) do |artifact, _|
artifact.file = fixture_file_upload(
Rails.root.join('ee/spec/fixtures/security_reports/master/gl-common-scanning-report-names.json'), 'application/json')
end
end
trait :container_scanning_feature_branch do trait :container_scanning_feature_branch do
file_format { :raw } file_format { :raw }
file_type { :container_scanning } file_type { :container_scanning }
......
{
"vulnerabilities": [
{
"category": "dependency_scanning",
"name": "Vulnerabilities in libxml2",
"message": "Vulnerabilities in libxml2 in nokogiri",
"description": "",
"cve": "CVE-1020",
"severity": "High",
"solution": "Upgrade to latest version.",
"scanner": {
"id": "gemnasium",
"name": "Gemnasium"
},
"location": {},
"identifiers": [],
"links": [
{
"url": "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1020"
}
]
},
{
"id": "bb2fbeb1b71ea360ce3f86f001d4e84823c3ffe1a1f7d41ba7466b14cfa953d3",
"category": "dependency_scanning",
"name": "Regular Expression Denial of Service",
"message": "",
"description": "",
"cve": "CVE-1030",
"severity": "Unknown",
"solution": "Upgrade to latest versions.",
"scanner": {
"id": "gemnasium",
"name": "Gemnasium"
},
"location": {},
"identifiers": [],
"links": [
{
"name": "CVE-1030",
"url": "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-1030"
}
]
},
{
"category": "dependency_scanning",
"name": "",
"message": "",
"description": "",
"cve": "CVE-2017-11429",
"severity": "Unknown",
"solution": "Upgrade to fixed version.\r\n",
"scanner": {
"id": "gemnasium",
"name": "Gemnasium"
},
"location": {
"file": "yarn/yarn.lock",
"dependency": {
"package": {
"name": "io.netty/netty"
},
"version": "3.9.1.Final"
}
},
"identifiers": [
{
"value": "2017-11429",
"type": "cwe",
"name": "CWE-2017-11429",
"url": "https://cve.mitre.org/cgi-bin/cwename.cgi?name=CWE-2017-11429"
},
{
"value": "2017-11429",
"type": "cve",
"name": "CVE-2017-11429",
"url": "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-11429"
}
],
"links": []
},
{
"category": "dependency_scanning",
"name": "",
"message": "",
"description": "",
"cve": "CWE-2017-11429",
"severity": "Unknown",
"solution": "Upgrade to fixed version.\r\n",
"scanner": {
"id": "gemnasium",
"name": "Gemnasium"
},
"location": {
"file": "yarn/yarn.lock",
"dependency": {
"package": {
"name": "io.netty/netty"
},
"version": "3.9.1.Final"
}
},
"identifiers": [
{
"value": "2017-11429",
"type": "cwe",
"name": "CwE-2017-11429",
"url": "https://cwe.mitre.org/cgi-bin/cwename.cgi?name=CWE-2017-11429"
},
{
"value": "2017-11429",
"type": "other",
"name": "other-2017-11429",
"url": "https://other.mitre.org/cgi-bin/othername.cgi?name=other-2017-11429"
}
],
"links": []
},
{
"category": "dependency_scanning",
"name": "",
"message": "",
"description": "",
"cve": "OTHER-2017-11429",
"severity": "Unknown",
"solution": "Upgrade to fixed version.\r\n",
"scanner": {
"id": "gemnasium",
"name": "Gemnasium"
},
"location": {
"file": "yarn/yarn.lock",
"dependency": {
"package": {
"name": "io.netty/netty"
},
"version": "3.9.1.Final"
}
},
"identifiers": [
{
"value": "2017-11429",
"type": "other",
"name": "other-2017-11429",
"url": "https://other.mitre.org/cgi-bin/othername.cgi?name=other-2017-11429"
}
],
"links": []
}
],
"remediations": [],
"dependency_files": [],
"scan": {
"scanner": {
"id": "gemnasium",
"name": "Gemnasium",
"url": "https://gitlab.com/gitlab-org/security-products/analyzers/gemnasium-maven",
"vendor": {
"name": "GitLab"
},
"version": "2.18.0"
},
"type": "dependency_scanning",
"start_time": "placeholder-value",
"end_time": "placeholder-value",
"status": "success"
}
}
...@@ -9,14 +9,62 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do ...@@ -9,14 +9,62 @@ RSpec.describe Gitlab::Ci::Parsers::Security::Common do
let(:artifact) { build(:ee_ci_job_artifact, :common_security_report) } let(:artifact) { build(:ee_ci_job_artifact, :common_security_report) }
let(:report) { Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago) } let(:report) { Gitlab::Ci::Reports::Security::Report.new(artifact.file_type, pipeline, 2.weeks.ago) }
let(:parser) { described_class.new } let(:parser) { described_class.new }
let(:location) { ::Gitlab::Ci::Reports::Security::Locations::DependencyScanning.new(file_path: 'yarn/yarn.lock', package_version: 'v2', package_name: 'saml2') }
before do before do
allow(parser).to receive(:create_location).and_return(nil) allow(parser).to receive(:create_location).and_return(location)
artifact.each_blob do |blob| artifact.each_blob do |blob|
parser.parse!(blob, report) parser.parse!(blob, report)
end end
end end
context 'parsing finding.name' do
let(:artifact) { build(:ee_ci_job_artifact, :common_security_report_with_blank_names) }
context 'when message is provided' do
it 'sets message from the report as a finding name' do
vulnerability = report.findings.find { |x| x.compare_key == 'CVE-1020' }
expected_name = Gitlab::Json.parse(vulnerability.raw_metadata)['message']
expect(vulnerability.name).to eq(expected_name)
end
end
context 'when message is not provided' do
context 'and name is provided' do
it 'sets name from the report as a name' do
vulnerability = report.findings.find { |x| x.compare_key == 'CVE-1030' }
expected_name = Gitlab::Json.parse(vulnerability.raw_metadata)['name']
expect(vulnerability.name).to eq(expected_name)
end
end
context 'and name is not provided' do
context 'when CVE identifier exists' do
it 'combines identifier with location to create name' do
vulnerability = report.findings.find { |x| x.compare_key == 'CVE-2017-11429' }
expect(vulnerability.name).to eq("CVE-2017-11429 in yarn.lock")
end
end
context 'when CWE identifier exists' do
it 'combines identifier with location to create name' do
vulnerability = report.findings.find { |x| x.compare_key == 'CWE-2017-11429' }
expect(vulnerability.name).to eq("CWE-2017-11429 in yarn.lock")
end
end
context 'when neither CVE nor CWE identifier exist' do
it 'combines identifier with location to create name' do
vulnerability = report.findings.find { |x| x.compare_key == 'OTHER-2017-11429' }
expect(vulnerability.name).to eq("other-2017-11429 in yarn.lock")
end
end
end
end
end
context 'parsing remediations' do context 'parsing remediations' do
it 'finds remediation with same cve' do it 'finds remediation with same cve' do
vulnerability = report.findings.find { |x| x.compare_key == "CVE-1020" } vulnerability = report.findings.find { |x| x.compare_key == "CVE-1020" }
......
...@@ -52,6 +52,42 @@ RSpec.describe Gitlab::Ci::Reports::Security::Identifier do ...@@ -52,6 +52,42 @@ RSpec.describe Gitlab::Ci::Reports::Security::Identifier do
end end
end end
describe '#cve?' do
let(:identifier) { create(:ci_reports_security_identifier, external_type: external_type) }
subject { identifier.cve? }
context 'when has cve as external type' do
let(:external_type) { 'Cve' }
it { is_expected.to eq(true) }
end
context 'when does not have cve as external type' do
let(:external_type) { 'Cwe' }
it { is_expected.to eq(false) }
end
end
describe '#cwe?' do
let(:identifier) { create(:ci_reports_security_identifier, external_type: external_type) }
subject { identifier.cwe? }
context 'when has cwe as external type' do
let(:external_type) { 'Cwe' }
it { is_expected.to eq(true) }
end
context 'when does not have cwe as external type' do
let(:external_type) { 'Cve' }
it { is_expected.to eq(false) }
end
end
describe '#to_hash' do describe '#to_hash' do
let(:identifier) { create(:ci_reports_security_identifier) } let(:identifier) { create(:ci_reports_security_identifier) }
......
...@@ -14,6 +14,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Locations::ContainerScanning do ...@@ -14,6 +14,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Locations::ContainerScanning do
let(:mandatory_params) { %i[image operating_system] } let(:mandatory_params) { %i[image operating_system] }
let(:expected_fingerprint) { Digest::SHA1.hexdigest('registry.gitlab.com/my/project:glibc') } let(:expected_fingerprint) { Digest::SHA1.hexdigest('registry.gitlab.com/my/project:glibc') }
let(:expected_fingerprint_path) { 'registry.gitlab.com/my/project:glibc' }
it_behaves_like 'vulnerability location' it_behaves_like 'vulnerability location'
......
...@@ -14,6 +14,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Locations::Dast do ...@@ -14,6 +14,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Locations::Dast do
let(:mandatory_params) { %i[path method_name] } let(:mandatory_params) { %i[path method_name] }
let(:expected_fingerprint) { Digest::SHA1.hexdigest('/some/path:GET:X-Content-Type-Options') } let(:expected_fingerprint) { Digest::SHA1.hexdigest('/some/path:GET:X-Content-Type-Options') }
let(:expected_fingerprint_path) { '/some/path' }
it_behaves_like 'vulnerability location' it_behaves_like 'vulnerability location'
end end
...@@ -13,6 +13,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Locations::DependencyScanning do ...@@ -13,6 +13,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Locations::DependencyScanning do
let(:mandatory_params) { %i[file_path package_name] } let(:mandatory_params) { %i[file_path package_name] }
let(:expected_fingerprint) { Digest::SHA1.hexdigest('app/pom.xml:io.netty/netty') } let(:expected_fingerprint) { Digest::SHA1.hexdigest('app/pom.xml:io.netty/netty') }
let(:expected_fingerprint_path) { 'pom.xml' }
it_behaves_like 'vulnerability location' it_behaves_like 'vulnerability location'
end end
...@@ -15,6 +15,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Locations::Sast do ...@@ -15,6 +15,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Locations::Sast do
let(:mandatory_params) { %i[file_path start_line] } let(:mandatory_params) { %i[file_path start_line] }
let(:expected_fingerprint) { Digest::SHA1.hexdigest('src/main/App.java:29:31') } let(:expected_fingerprint) { Digest::SHA1.hexdigest('src/main/App.java:29:31') }
let(:expected_fingerprint_path) { 'App.java' }
it_behaves_like 'vulnerability location' it_behaves_like 'vulnerability location'
end end
...@@ -15,6 +15,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Locations::SecretDetection do ...@@ -15,6 +15,7 @@ RSpec.describe Gitlab::Ci::Reports::Security::Locations::SecretDetection do
let(:mandatory_params) { %i[file_path start_line] } let(:mandatory_params) { %i[file_path start_line] }
let(:expected_fingerprint) { Digest::SHA1.hexdigest('src/main/App.java:29:31') } let(:expected_fingerprint) { Digest::SHA1.hexdigest('src/main/App.java:29:31') }
let(:expected_fingerprint_path) { 'App.java' }
it_behaves_like 'vulnerability location' it_behaves_like 'vulnerability location'
end end
...@@ -37,6 +37,14 @@ RSpec.shared_examples 'vulnerability location' do ...@@ -37,6 +37,14 @@ RSpec.shared_examples 'vulnerability location' do
end end
end end
describe '#fingerprint_path' do
subject { described_class.new(**params).fingerprint_path }
it "generates expected fingerprint" do
expect(subject).to eq(expected_fingerprint_path)
end
end
describe '#==' do describe '#==' do
let(:location_1) { create(:ci_reports_security_locations_sast) } let(:location_1) { create(:ci_reports_security_locations_sast) }
let(:location_2) { create(:ci_reports_security_locations_sast) } let(:location_2) { create(:ci_reports_security_locations_sast) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment