Skip to content

Automatically resolve vulnerabilities when a SAST rule is disabled or removed

Summary

Sec section analyzers should include metadata on the outcomes of scans in order to inform report ingestion.

This data can be used to direct the processing of reports for more efficient vulnerability management.

Goals

Implement a framework where analyzer reports can influence their parsing.

Challenges

  • Clear delineation between analyzer responsibilities, vulnerability management behavior, automation, and user expectations
  • Analyzers should remain simple and stateless without understanding the full scope of past and future behavior (which rules were previously enabled)
  • Increased artifact size due to added metadata

Opportunity

We want to continue to iterate on our analyzer prepacked rulesets by adding, removing, and modifying rules without causing significant data issues within user projects.

At present when a finding is no longer detected, we lack granularity in clearly differentiating between the potential outcomes and communicating whether the resolution is due to rule refinement, analyzer behavior, or external factors (such as the relevant code being addressed within the repository).

To better distinguish between these cases we must:

  1. Build a framework to more clearly inform the relationship between analyzers and their parsers
  2. Build usage guidelines around how findings should be processed in relation to their rule states

The scenarios to be handled include:

  1. Post-analyzer identifies a false-positive
  2. Analyzer drops noisy rule (i.e. one prone to FPs)
  3. SAST Custom Rulesets disables prepackaged rule

The actions taken by these scenarios cover:

  1. Identifying false-positives
  2. Resolving false-positives
  3. Identifying high-noise rules
  4. Resolving high-noise rules

Framework to define and enforce limits

Our analyzer reports are currently stateless with no knowledge of past executions.

For comparisons between different repository states (such as source and target branches), the current ingestion flow must calculate set differences between added, removed, and unchanged vulnerabilities.

By introducing two levels of report enrichment we can include metadata around both vulnerabilities and scan executions to better inform our parsing logic. At the finding level, this is already supported via vulnerability.flags\[\]. At the scan execution level, there is no existing mechanism.

The existing vulnerability.flags\[\] framework is flexible in providing identifier data on top of existing findings to inform processing. This is currently limited to attaching alerts to identify findings as false-positives, but could be expanded to take actions, as well.

To cover usecases in which analyzers require a more holistic view of rule coverage, a new scan.primary_identifiers\[\] field must be introduced to allow analyzers to include the full list of scanned rules.

Relationship to Scan Result Policies

The existing Scan Result Policies feature is a similar automation framework, however the focus is currently around user-driven policy actions. The goal of the analyzer automation flow is to provide an analyzer-driven approach in which outcomes are driven by result refinements rather than user business needs.

Limitations of vulnerability.flags\[\]

There are performance implications in relying solely on flags. Since flags must attach to specific findings we must include the full result-set regardless of whether we wish to discard a portion. This bloats reports and has performance implications by relying on post-filtering of results.

In the event of a noisy rule that produces many insignificant findings, it is inefficient to include those findings in the report simply to be excluded later. Further, in some cases the scanner may fail or experience slowdown by including such rules, such as this issue.

Inclusion of exhaustive rule identifiers in scan.primary_identifiers

By including a comprehensive list of scanned rules in scan.primary_identifiers we can provide the parser with a means of identifying past and present rulesets generating vulnerability results. This would allow the parser to identify existing findings associated with rules that are no longer being included in analyzer runs, such as the removal of a rule category.

By segmenting which findings are no longer applicable instead of no longer detected we can more accurately communicate which findings can be auto-resolved and which should require manual review.

Usage guidelines for flags vs parsing behaviors

TODO

Example 1: Post-analyzer processing identifies a false-positive

This work has already been implemented in %14.2 as part of VET FP elimination.

  1. brakeman-sast runs and returns a false-positive finding in vulnerability report
  2. vet post-analyzer runs in same pipeline and attaches flag to report identifying specific finding as false-positive
  3. Vulnerability page displays alert marking finding as false-positive
graph LR

repository --> brakeman_sast

subgraph post-analyzer pipeline
  brakeman_sast --> report1("gl-sast-report.json")

  report1 --> vet::verify

  vet::verify --> reportout1("gl-sast-report.json")
end

subgraph ingestion
  reportout1 --> ingestsvc["Security::Ingestion::IngestReportsService"]

  ingestsvc-- persist flags --> flagtask["Security::Ingestion::Tasks::IngestVulnerabilityFlags"]
end

Example 2: Post-analyzer processing eliminates a false-positive

This scenario is closely aligned with the existing false-positive identification flow, but involves an automated action

  1. brakeman-sast runs and returns a false-positive finding in vulnerability report
  2. vet post-analyzer runs in same pipeline and attaches flag to report identifying specific finding as false-positive
  3. Vulnerability page displays alert marking finding as false-positive
  4. Vulnerability page changes vulnerability status to "resolved" with note that an automated action was performed
graph LR

repository --> brakeman_sast

subgraph post-analyzer pipeline
  brakeman_sast --> report1("gl-sast-report.json")

  report1 --> vet::verify

  vet::verify --> reportout1("gl-sast-report.json")
end

subgraph ingestion
  reportout1 --> ingestsvc["Security::Ingestion::IngestReportsService"]

  ingestsvc --> flagtask1["Tasks::IngestVulnerabilityFlags"] --> flagtask2["Security::Ingestion::Tasks::UpdateVulnerabilityStatus"]
end

Example 3A: Analyzer drops noisy rule (prone to FPs)

  1. semgrep-sast is updated to remove rule that has been identified as overly noisy and prone to producing high false-positive results
  2. Report includes list of rules executed during scan
  3. Report parser identifies all existing findings that are not included in list of rules and marks as auto-resolved due to rule no longer being included
graph LR

repository --> brakeman_sast

subgraph analyzer pipeline
  brakeman_sast --> report1a["noisy-rule-123 dropped"]
  report1a --> report1b["scan.primary_identifiers populated"]
  report1b --> reportout1("gl-sast-report.json")
end

subgraph ingestion
  reportout1 --> ingestsvc["IngestReportService"]

  ingestsvc--> flagtask1["Tasks::MarkDroppedAsResolved"]
end

Alternative approach using flags

graph LR

repository --> brakeman_sast

subgraph analyzer pipeline
  brakeman_sast --> report1a["noisy-rule-123 dropped"]
  report1a --> report1b["scan.primary_identifiers populated"]
  report1b --> reportout1("gl-sast-report.json")
end

subgraph ingestion
  reportout1 --> ingestsvc["IngestReportsService"]

  ingestsvc-- generate flags based on identifiers diff --> flagtask1
  flagtask1["Tasks::IngestVulnerabilityFlags"] --> flagtask2
  flagtask2["Tasks::UpdateVulnerabilityStatus"]
end

Example 3B: Using two analyzers, one drops noisy rule (prone to FPs)

  1. spotbugs-sast is used returning kotlin results (kotlin-rule-1)
  2. semgrep-sast is updated to remove semgrep-rule-1 rule that has been identified as overly noisy and prone to producing high false-positive results
  3. Report includes a partial list of rules executed during scan (those belonging to semgrep)
  4. Report parser merges result-sets, identifies all existing semgrep findings that are not included in list of rules and marks as auto-resolved due to rule no longer being included
  5. spotbugs results are unaffected
graph LR

repository --> semgrep_sast

subgraph analyzer pipeline
  semgrep_sast --- report1a1["DROP semgrep-rule-1"]
  semgrep_sast --- report1a2["SCAN semgrep-rule-2"]
  report1a2 -- "primary_identifiers: [semgrep-rule-2]" --> reportout1("gl-sast-report.json")

  spotbugs_sast --- report2a["SCAN kotlin-rule-1"]
  report2a -- "primary_identifiers []" --> reportout2("gl-sast-report.json")
end

subgraph ingestion
  reportout1 --> MergeReports["Merged identifiers: [semgrep-rule-2]"]
  reportout2 --> MergeReports

  MergeReports --> ingestsvc["..."]
  ingestsvc--> MarkDroppedAsResolved
  MarkDroppedAsResolved -- "resolves only vulns where ident match semgrep" --> MarkDroppedAsResolved
end

Principles

  1. Avoid duplicating behavior in Scan Result policies
  2. Avoid duplicating behavior in auto-resolution mechanisms within vulnerability management
  3. Avoid duplicating behavior in user-driven vulnerability management
  4. Build framework in a way that defines actions analyzers can take to handle existing data

Background

As proposed by @twoodham this pattern could be very useful for handling rule changes going forward.

Excerpts from Slack thread set to expire 2022-10-08

@twoodham Wild idea: think about including identifiers of rules used by the analyzer as part of its operation. That could then inform report handling - and maybe auto-resolve of vulnerabilities with identifiers which aren't part of the reported set.

that's a great way of handle future pain. I swear we had that in a collective issue from before but I could only find the "enriching" one above. It came up during FP-elimination work. We have the scaffolding already with vulnerability_flags https://gitlab.com/gitlab-org/security-products/security-report-schemas/-/blob/8caade7ba7575579b99d9c4be3401ad14757a2e6/src/security-report-format.json#L506-512, so it should be a fairly small lift to implement a "Do X based on Flag Y" parsing step

I'm thinking it could play into report parsing. Existing findings, if no longer detected, could be auto-resolved if their identifier wasn't part of the new report’s metadata.

Status

Edited by Lucas Charles