MR detects new, fixed vulnerabilities when switching to non-DinD setup
Summary
The Security Scanning widget detects new, fixed vulnerabilities in a merge request that switches Dependency Scanning configuration from Docker-in-Docker setup to non-DinD setup.
As a side-effect, vulnerability feedback is lost.
This issue might affect SAST as well. See discussion.
Further details
The DinD orchestrator loops over the Docker images of the Dependency Scanning (DS) analyzers, in the order defined in DS_DEFAULT_ANALYZERS
in the configuration template. This order has an impact when merging the reports using issue.MergeReports, which in turn delegates to issue.Dedupe.
In the non-DinD setup, the order in which DS reports are processed is not deterministic,
so the Rails backend might generate a different result when removing the duplicates, even though the logic is the same. The deduplication logic was introduced in 887812d6
Steps to reproduce
Create a project where Docker-in-Docker Dependency Scanning is enabled in the master
branch, and create a MR that switches to non-DinD Dependency Scanning.
Example Project
gitlab-org/security-products/tests/ruby-bundler!80 (closed)
What is the current bug behavior?
The Security Scanning widget lists new vulnerabilities and fixed vulnerabilities.
Security scanning detected 11 new, and 9 fixed vulnerabilities
New vulnerabilities:
Unknown Untrusted Search Path in ffi
Unknown Improper Input Validation in nokogiri
Unknown Access of Resource Using Incompatible Type (Type Confusion) in nokogiri
Unknown Improper Input Validation in nokogiri
Unknown Use After Free in nokogiri
Unknown Allocation of Resources Without Limits or Throttling in puma
Unknown Cross-site Scripting in rack
Unknown Information Exposure in rack
Unknown Uncontrolled Resource Consumption in rack
Unknown Cross-site Scripting in sinatra
Unknown Path Traversal in sinatra
Fixed vulnerabilities:
High ruby-ffi DDL loading issue on Windows OS
Medium sinatra ruby gem path traversal via backslash characters on Windows
Unknown Nokogiri gem, via libxml, is affected by DoS and RCE vulnerabilities
Unknown Nokogiri gem, via libxml2, is affected by multiple vulnerabilities
Unknown Revert libxml2 behavior in Nokogiri gem that could cause XSS
Unknown Nokogiri gem, via libxslt, is affected by multiple vulnerabilities
Unknown Possible DoS vulnerability in Rack
Unknown Possible XSS vulnerability in Rack
Unknown XSS via the 400 Bad Request page
What is the expected correct behavior?
The Security Scanning widget lists no vulnerabilities because the MR introduces no new vulnerabilities, and fixes no existing ones.
Possible fixes
A quick hack would be to update the MergeReportsService
to sort the vulnerability findings/occurrences prior to removing the duplicates, using the current value of DS_DEFAULT_ANALYZERS
as a source of truth. See Dependency-Scanning.gitlab-ci.yml:
DS_DEFAULT_ANALYZERS: "bundler-audit, retire.js, gemnasium, gemnasium-maven, gemnasium-python"
We could do the same for SAST.
Ultimately we should improve the deduplicate logic so that MergeReportsService
does not depend on a hard-coded list.
Ideally, the Rails backend would store all the vulnerability findings, and deduplicate them when rending the list for the UI.
Implementation Plan
Hard code priority to scanners and sort them something like following
def sort_by_ds_analyzers
analyzer_priorities = {
"bundler_audit" => 1,
"retire.js" => 2,
"gemnasium" => 3,
"gemnasium-maven" => 3,
"gemnasium-python" => 3
}.freeze
@source_reports.sort! do |a, b|
a_scanner_id, b_scanner_id = a.scanners.values[0].external_id, b.scanners.values[0].external_id
analyzer_priorities[a_scanner_id] <=> analyzer_priorities[b_scanner_id]
end
end
Orchestrator layer was deduping reports in certain order. In that way we can imitate behaviour of orchestrator layer and have same output when switching no-dind.
Availability and Testing
SET should add an end to end test to support non-DinD branches.