Reduce number of CVE collisions dependency-check/dependency-check-son… #763
Conversation
|
Can you please provide a screenshot of how the offset affects the SonarQube UI? |
|
Please let me know if this isn't what you meant, or if you'd like any other screenshots or information. |
There was a problem hiding this comment.
I think that I don't like the solution. If I'm looking over the changes correctly, then take the vulnerability e.g. CVE-2022-23305, make it 202223305 and use this number as a direct text offset (202223305) or as a text offset + 1 (202223306). Do I understand the approach correctly?
| putDependencyMap(dependency, new TextRangeConfidence(buildGradle.selectLine(linenumber), Confidence.HIGHEST)); | ||
| putDependencyMap(dependency, vulnerability, new TextRangeConfidence(buildGradle.selectLine(linenumber), Confidence.HIGHEST)); | ||
| return; | ||
| } else { |
There was a problem hiding this comment.
The else is unnecessary because we have a return in the if.
...dependency-check-plugin/src/main/java/org/sonar/dependencycheck/reason/DependencyReason.java
Outdated
Show resolved
Hide resolved
|
Yes, with the result of the issue being attached to the lines at offset 202223305, instead of the entire line, allowing for multiple vulnerabilities on the same line without colliding. It's certainly not a complete solution, since if the vulnerability has no numbers in the name it will default to offset 0, allowing for collisions. Or if one line has multiple vulnerabilities with the same CVE. Otherwise, nothing changes visually, just the location that the issues are tracked internally by SonarQube. |
Reamer
added
lifecycle/frozen
|
I don't like the solution with the offsets because it has an unclean taste. Since the solution probably works, I leave the pull request open for people who also want to solve the problem. |
…ar-plugin#682
Uses the CVE number as a line offset to reduce overlap.