Enrich SAST report with more metadata
Description
The current MR widget shows a simple list of vulnerabilities reported by SAST tools with:
- Priority
- Title
- Affected file path
This can be easily enriched to provide more useful data to the users.
Priority
property is currently mixing several different meanings from different tools:
1) The - Bandit: severity
- Brakeman: confidence
- Find Sec Bugs: rank
- Flawfinder: risk level
- Go AST Scanner: severity
There is a semantic work here and we need to check the tools are using the correct word regarding the associated value they provide.
At least severity
and confidence
are something different. A vulnerability could have a Low
confidence level
(unlikely to impact your project) but a High
severity (can cause a lot of damages).
I'm not sure mixing these into a single Priority
property is meaningful for our users.
Also, we could leverage existing industry standard like The Common Vulnerability Scoring System (CVSS) to provide details about the severity (when available).
2) There are a lot more information available (or potentially available) that we can display
We could definitely improve the UX by adding more data. E.g. with current state, a user that sees the report has to:
copy/paste the vulnerability title into google to find out what it isLook into the affected file (no line provided) for the corresponding vulnerability
This could be easily enhanced by embedding a short description and/or providing a link to the vulnerability source or the CVE database, etc.
And providing the corresponding line in the impacted file for all tools could save a good amount of time too.
With #5151 (closed) we added a modal that allows to display a lot of useful information. We can now iterate on this to enrich the display.
Proposal
-
1. Gather all the metadata available from the different SAST tools -
2. Normalize the output for common data to be shown by default in the list:
Severity
(Confidence
):Title
inFile
-
3. Add as much valuable data as possible to the modal
1. Available data
Here is a list of the available properties for each tools we currently rely on:
Property \ Tool | Bandit | Brakeman | Find Sec Bugs | Flawfinder | Go AST Scanner |
---|---|---|---|---|---|
severity | |||||
title | |||||
file | |||||
start line | |||||
end line | |||||
external id (e.g. CVE) | |||||
urls | |||||
internal doc/explanation | |||||
solution | |||||
confidence | |||||
affected item (e.g. class or package) | |||||
source code extract | |||||
internal id | |||||
date | |||||
credits |
-
✅ => we have that data -
⚠ => we have that data but it's partially reliable, or we need to extract that data from unstructured content -
❌ => we don't have that data or it would need to develop specific or inefficient/unreliable logic to obtain it.
Here is a list of tools values for severity
and confidence
properties:
- severity:
- Bandit: UNDEFINED, LOW, MEDIUM, HIGH
- Find Sec Bugs (rank): OF_CONCERN, TROUBLING, SCARY, SCARIEST
- Go AST Scanner: UNDEFINED, LOW, MEDIUM, HIGH
- confidence:
- Bandit: UNDEFINED,LOW,MEDIUM,HIGH
- Brakeman: Weak, Medium, High
- Find Sec Bugs: Unknown, Experimental, Low, Medium, High
- Flawfinder(risk level): 0 to 5, the higher the more risky
A word on "hybrid" tool Brakeman
:
Brakeman is a SAST tool focused on the RoR framework. As RoR is published as a rubygem it is also a dependency. This ends up having some vulnerabilities reported as dependency vuln (CVE 123-456) by other dependency scanners tools on top of Brakeman. As far as a common identifier (like CVE) is provided we can dedupe them but this means both tools report must go through a common report generator/filter. Food for thought: this also means that this aggregation/filtering logic will have to reside in CI backend code if the reports are generated by separate jobs.
Here here the list of mapped properties for each tool + some relevant comments to save time when implementing:
Property \ Tool | Bandit | Brakeman | Find Sec Bugs | Flawfinder | Go AST Scanner |
---|---|---|---|---|---|
severity | issue_severity | rank | severity | ||
title | issue_text | message | ShortMessage, LongMessage | Warning | details |
file | filename | file | SourceLine@sourcepath src/maim/java dir |
File | file |
start line | line_number | line | SourceLine@start | Line | line |
end line | calculate with line_range | SourceLine@end | |||
external id (e.g. CVE) |
|
|
CWEs | ||
urls | link (sometimes link to warning type page) |
|
|||
internal doc/explanation | issues type list (incomplete) | warning types | bug patterns list | ||
solution |
|
Suggestion | |||
confidence | issue_confidence | confidence | priority | :Level: | confidence |
affected item | location (contains: type, class, method) | Class, Method | Name (method?) | ||
source code extract | code | code | Context | code | |
internal id | test_id | warning_code, check_name, fingerprint | abbrev, category, type | Fingerprint, Category | rule_id |
date | |||||
credits |