Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue description being changed to different CVE on repeated runs #682

Open
jordannstrong opened this issue Jul 26, 2022 · 14 comments
Open
Assignees
Labels
bug lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@jordannstrong
Copy link

Describe the bug
After addressing vulnerabilities in a previous analysis' report, some vulnerability issues within SonarQube are having their descriptions updated with the descriptions of other vulnerabilities that are present in the report. In some cases, issues that had been manually marked with a resolution had the resolution removed, making them appear as if they had never been resolved. Comments, tags, and other features that are managed directly by SonarQube remain the same, only the description and severity are changed to that of another issue.

To Reproduce

  1. Perform an analysis on a project that results in vulnerabilities.
  2. Resolve at least one of these vulnerabilities in code.
  3. Perform a second analysis.
  4. Some issues will have had their descriptions swapped, and/or their resolved status changed.

Current behavior
SonarQube vulnerability issue descriptions and severities are being mixed up on subsequent analyses.

Expected behavior
Each issue's description and severity should remain the same once the issue has been created.

Screenshots
I used comments to track the issues, by commenting the CVE that the issue was for after the first analysis. After the second analysis, it had changed from the one in the comment to the one in the description.
image

Versions (please complete the following information):

  • dependency-check: 6.5.1
  • sonarqube: 9.5.0.56709
  • dependency-check-sonar-plugin: 3.0.1

Additional context
This appears to be similar to #55.

@Reamer
Copy link
Member

Reamer commented Jul 27, 2022

Could you please provide the log output.

@jordannstrong
Copy link
Author

sonarqube_ce - Redacted.log

Please let me know if any other logs or information is needed. Thank you.

@jordannstrong
Copy link
Author

Some additional info: I noticed that all of the issues were associated to one line in the POM in SonarQube, because they were all Spring managed transitive dependencies. When I broke them out and explicitly listed each dependency, each of the issues corresponded to the line that the dependency they were associated with was listed on, as expected. I then updated some of the libraries in a way that would address some of the vulnerabilities for that dependency, but not all of them. The result was that the issues that were associated with dependencies that weren't updated remained correct. However, the ones associated with updated libraries were mixed up, but only among themselves. For example, issues associated with spring-core and tomcat-embed-core were mixed up, but the CVEs that were mixed up for spring-core remained associated with spring-core, and the same for tomcat-embed-core.

Looking at the fix for #55 in #61, it seems like the plugin matches issues based on the line it's on, but if multiple issues are associated with the same line, then there's no guarantee that the issues will be updated in the correct order.

@Reamer Reamer self-assigned this Jul 28, 2022
@Reamer
Copy link
Member

Reamer commented Jul 28, 2022

Please let me know if any other logs or information is needed. Thank you.

That logs are from the SonarQube-Process. I need the debug log of the SonarQube-Agent.

The plugin does not resolve the transitive dependencies from your project file. To do this, you would have to ship Maven, Gradle, NPM, etc. inside the jar. This is simply unrealistic.
Instead, it looks in the project file for the GroupId, ArtefactId, and Version to assign the vulnerable dependency (SonarQube Issue) to the correct line if possible.
With NPM this is quite easy, because the package-lock.json also lists the transitive dependencies and we link to this file.
If no position was found within the file, we link to the first line. This is still better than linking the issue against the SonarQube project.
The order here is apparently not deterministic, which could cause your bug.

I will be on vacation for the next three weeks, so I will not be able to continue working on your issue until after that.

@jordannstrong
Copy link
Author

SonarScanner Debug First Scan.txt
SonarScanner Debug Second Scan.txt

Sorry about that, I believe these are the correct logs.

The order here is apparently not deterministic, which could cause your bug.

Yes, this seems to be it. To summarize my current understanding: since a single library can have multiple vulnerabilities, there's the potential for multiple issues to be linked to the same line. When there are, if the order changes for some reason, such as one of the vulnerabilities being resolved and no longer appearing in the dependency-check report, the vulnerabilities might be updated to the wrong issues.

@piotrekfilip
Copy link

My two cents, SonarQube uses ruleKey to match existing issues (https://docs.sonarqube.org/latest/user-guide/issues/#header-3, https://github.com/SonarSource/sonarqube/blob/9.5.0.56709/sonar-core/src/main/java/org/sonar/core/issue/tracking/AbstractTracker.java), so I think one of the ways to solve this would be to use ExternalIssues, similarly as
https://github.com/SonarSource/sonarqube/blob/9.5.0.56709/sonar-scanner-engine/src/main/java/org/sonar/scanner/externalissue/ExternalIssueImporter.java#L71

Each vulnerability could be created with different AdHocRule, with ruleId/ruleKey set to e.g. CVE ID, which should allow SonarQube to correctly match existing issues.

However this also have drawbacks, for example it is not possible to set ExternalIssue as False Positive within SonarQube, also background tasks may take a longer time (e.g. due to 'Persist new ad hoc Rules' step).

@github-actions
Copy link

github-actions bot commented Oct 4, 2022

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 4, 2022
@Reamer
Copy link
Member

Reamer commented Oct 4, 2022

I tried using ExternalIssue (https://github.com/dependency-check/dependency-check-sonar-plugin/tree/external_issue) and took a fresh look at AdhocRules, but unfortunately the functionality is limited compared to the current solution.

AdHocRules have neither a tag nor a CWE nor an owasptop10 status. https://github.com/SonarSource/sonar-plugin-api/blob/master/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/rule/AdHocRule.java

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 5, 2022
@github-actions
Copy link

github-actions bot commented Dec 5, 2022

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 5, 2022
@gabrimonfa
Copy link

I'm also experiencing this issue with transitive dependencies in pom file, that are all associated to the file, without line number.
Their comments and status changes randomly in different runs.
If I understand correctly a possible solution would be to have a predictable order of the issues.
What about order them by CVE number? It will add some predictability.

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 16, 2022
@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 14, 2023
jordannstrong pushed a commit to jordannstrong/dependency-check-sonar-plugin that referenced this issue Feb 24, 2023
@jordannstrong
Copy link
Author

jordannstrong commented Feb 24, 2023

Using the CVE number was a good idea. The solution I came to was to use it as a line offset, so that the issue would be associated with the line, but only at the column that matches the CVE number, so that the potential for collisions on the same line is reduced. I've only been able to test it on a Maven project, so I'm not sure if these changes would affect the Gradle or NPM scanners in any way, but it at least seems to be working correctly there.

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 25, 2023
@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 27, 2023
@Reamer Reamer added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Apr 27, 2023
@mlemmens
Copy link

mlemmens commented Feb 5, 2024

We are also experiencing this issue.

After running the analysis for the second time a lot of issues have been closed by SonarQube, but the vulnerabilities are still there, only new issues have been created.

For example CVE-2023-2976, a Guava vulnerability, was first created on line x. After running the analysis again a new issue about the same Guava vulnerability has been created on line y and the first one has been closed. The line number was removed.

According to the HTML report the Guava dependency is a transitive dependency of the following dependencies that are listed in our build.gradle file.

Report:

pkg:maven/com.google.http-client/google-http-client-jackson2@1.42.0
pkg:maven/com.google.oauth-client/google-oauth-client@1.34.1

build.gradle:

implementation "com.google.oauth-client:google-oauth-client:1.34.1"
implementation "com.google.http-client:google-http-client-jackson2:1.42.0"

However after running the analysis again, the order in the report has changed. I think this is the cause of issues being closed and recreated.

Versions

  • dependency-check: 8.2.1 (we still need to upgrade to 9)
  • sonarqube: 9.9.3.79811
  • dependency-check-sonar-plugin: 4.0.1

Update 14-02-2024: also with OWASP dependency check version 9.0.9 we have this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

5 participants