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

Sonar mixes up issue status after repeated runs #55

Closed
klemens-mang opened this issue Mar 6, 2018 · 24 comments
Closed

Sonar mixes up issue status after repeated runs #55

klemens-mang opened this issue Mar 6, 2018 · 24 comments

Comments

@klemens-mang
Copy link

Using the owasp-dependency-check via the Maven plugin and submitting the results into sonar via this plugin gives me some weird issues.

On each run of the Maven build the Sonar plugin mixes up the reported vulnerabilities with already existing ones. One could observe this phenomenon when some of the found vulnerabilities were marked with a different status or augmented with comments. After the Maven build some of the changed status values and comments are then associated with different (existing) issues.

I was debugging this issue by looking at the data stored in my local Sonar postgres database. Sonar is storing the status values in the table 'issues'. After another run of the Maven build message (description) values of existing issues is updated with another issue's message. The prior status value, however, stays the same. I can reproduce this issue easily when running:

$ mvn sonar:sonar

I'm using:

  • Apache Maven 3.5.2
  • dependency-check-maven 3.1.1
  • dependency-check-sonar-plugin 1.1.0 (installed via SonarQube Marketplace)
  • SonarQube 7.0

Did anyone else experience something like this before?

Thx

@klemens-mang klemens-mang changed the title Sonar mixes up issue status after repeated run Sonar mixes up issue status after repeated runs Mar 6, 2018
@spindel87
Copy link

I have the same issue, its not the same setup but the result is the same. Issuse are mixed up resolved are changed to not resovled and the other way around.

Diffrence here is that Im using TFS and using SonarQube 6.7

@klemens-mang
Copy link
Author

@stevespringett as you seem to be the main maintainer for this project I would kindly ask you for your input to this issue. Is this a bug or a feature that is not yet implemented? If the latter is true I could try to work on some contribution in order to implement the feature.

@stevespringett
Copy link
Contributor

Most SQ plugins add issues on the files they analyze. The Dependency-Check plugin does not do this, since in many cases, the dependencies are outside the source directory being scanned (i.e. .m2) or may not be present at all.

Therefore the Dependency-Check SQ plugin adds issues to the module level and does so using the SQ API as seen here.

https://github.com/stevespringett/dependency-check-sonar-plugin/blob/master/sonar-dependency-check-plugin/src/main/java/org/sonar/dependencycheck/DependencyCheckSensor.java#L77-L84

This code is called by iterating through all the dependencies in the dependency-check-report and then for each dependency, iterating on all the vulnerabilities for that specific dependency. This results in issues being created in SQ on the module for each vulnerability in the DC report.

Comments, analysis decisions, table storage, etc are outside the scope of what this plugin is responsible for. That logic is internal to SQ. While I don't want to overreach and say that it's a SQ problem, there's a good possibility it is.

I'm prepping for a big launch in a few weeks on another opensource project I'm working on followed by a series of interviews and launch-related activities. I likely will not be able to dig into this until mid-April at the earliest. But if it does turn out to be an issue with this plugin, PR's are always welcome 😁

@klemens-mang
Copy link
Author

Hi @stevespringett,
thanks for your quick and extensive reply. If the plugin is working as you're describing it could actually be an issue of SQ itself.
I checked my local SQ database and understood that each time the mvn sonar:sonar goal is executed (without any code change) all existing issue entries, their so-called "keys" and all SQ issue metadata (responsible person, status, etc.; as declared in SQ database) are staying the same. The issue, however, is that the contents generated by the plugin (affected dependency name, vulnerability description, etc.) is shuffled among the existing issue entries.

I hope I'll have some time to look into this issue in more detail. Once I have gathered more details I'll let you know.

@sschober
Copy link
Contributor

sschober commented May 3, 2018

I am also witnessing the behaviour described by klemens-mang, using SonarQube 6.7.0 and version 1.1.0 of the dependency-check-sonar-plugin.

Looking at the SonarQube code for Version 6.7.3 (Tacker.java#L30), I think the problem might be the matching strategies (LineAndLineHashKey, LineHashAndMessageKey, ...), which are leaning heavily on the existence of a line number on an issue.

Your code does not seem to set such a line number (which line number should it choose, as you explain, you set the issue on the module). In that case, I think the first matching strategy, LineAndLineHashKey matches for each combination of issues, as Issue.line is always null and Object.equals() returns true if both elements are null. So, in essence, it's nondeterministic, which newly detected issue is matched against which already found one.

This theory seems to explain the described behaviour and I am currently experimenting with a work-around (assigning the issue not to the module, but to the input file of the dependency and choosing a line number based on the CVE number). But, I think, this approach is just a work around. Really, the matching strategies in sonar should be fixed, or, the API should make it clear, that line numbers are mandatory. What do you think?

@sschober
Copy link
Contributor

sschober commented May 4, 2018

you can see this idea in action in PR #61.

@dzmitry-rudnouski
Copy link

Reproduced with dependency-check-sonar-plugin 1.1.0 and SonarQube 7.1.
@stevespringett, kindly ask you to look into PR above.

stevespringett added a commit that referenced this issue Aug 6, 2018
@stevespringett
Copy link
Contributor

@sschober I also think the underlying SonarQube matching strategy code likely needs changed. This is certainly a workaround (which appears to do the job - thanks @sschober), but I do think SQ needs to do some cleanup - or at a minimum remove the ability to create issues without a line number since that seems to mess everything up. This is likely an issue introduced between SQ4 and SQ7, as I do not remember older SQ versions having this issue.

@Kovshar-OS
Copy link

Hello! Do you have solution for this problem? I have same mess in SQ 7.0 - my comments on issues in SQ are moving to other issues, and statuses are changed from opened to fixed. And some old issues became as new issues. I see it was solution in f5a1775 but it was revrted in 34f1c3a. Now I use sonar-dependency-check-plugin-1.1.3. and dependency-check-4.0.0 CLI. So, any help will be appretiated!

@Reamer
Copy link
Member

Reamer commented Jan 14, 2019

Hi @Kovshar-OS,
we try to link issues to project configuration file (pom.xml, gradle.build ...). Report files should not be uploaded to sonar.
At the moment it's a work in progress, see #99
If you have other projects then gradle or maven, i would appreciate, when you provide a very simple example project in our example folder.
You can also test the WIP merge request #99 and return feedback. Maybe it solves already your problem.
Best Regards
Reamer

@Kovshar-OS
Copy link

Hi, Reamer! Thank you for response!
I'v compile sonar-dependency-check-plugin-1.2.2.jar plugin from sources in #99 and try to run it under SonarQube 7.0. For some reason( maybe version of SonarQube) it does't work (SonarQube falling dump when starting). So I returned to sonar-dependency-check-plugin-1.1.3.jar

Two words about my projects - both types - maven and gradle and I have a folder .git\lib with dependencies for projects.

But! I don't use maven or gradle scanner for project (I use sonar-scanner and depndency-check command line). So, can it be real to make issues from dependencies files(hash md5 etc)?

@Reamer
Copy link
Member

Reamer commented Jan 14, 2019

Hi @Kovshar-OS,
you are right the merge request #99 doesn't work with Sonarqube 7.0. I uploaded a new branch for Sonarqube 6.7 and above. This branch should work with your Sonarqube version.

I think it's no classic project setup with dependencies in .git/lib folder. In most cases your dependencies are downloaded with maven or gradle during build and then located somewhere else in your filesystem (definitely outside of git Repository). For example maven dependencies are located in ~/.m2/repository.

This plugin should be independently from sonar-scanner, gradle or maven. It using the generated reports from dependency-check (how ever they are created) and creates some issues and metrics in sonarqube.

At the moment Sonarqube has problem to identify issues which are linked to a maven module or a file without line number. See comment #55 (comment)

@Kovshar-OS
Copy link

Hello Reamer! Thank you for response!
With new version of code, that you gave, I can succesfully run SonarQube. But situation with shifting key of issues in data base stay the same. Please look at the screenshot that I attached.
issue_key_shifting

@Reamer
Copy link
Member

Reamer commented Jan 17, 2019

Hi @Kovshar-OS,
are the issues linked against module, file or file with line number?

@Kovshar-OS
Copy link

Hi Reamer, can you explain how can I get this information.
(In database in Issues table column Line is null for created\changed dependency issues).

@Kovshar-OS
Copy link

Hi, Reamer! Do you have some good news about fixing issue? Thank you!

@Reamer
Copy link
Member

Reamer commented Feb 11, 2019

Hi @Kovshar-OS,
thanks for testing the code. I had a few weeks off, sorry for late response.

You should see this information in your sonarqube dashboard.
Here some picture with all three cases:

Issue linked against file
link_against_file

Issue linked against file with line number
link_against_file_with_line

Issue linked against module
link_against_module

If I have enough time, I will fix the bug, so dependency issues are always linked to files with a line number. I'll let you know when the code is ready.

@Kovshar-OS
Copy link

Hi, Reamer! Thank you for response! I will wait for fix. Thank you again!

@rady66
Copy link

rady66 commented Feb 27, 2019

Hopefully, that will get fixed soon as it is really crucial from CI/CD and security/release management perspectives.

@Reamer
Copy link
Member

Reamer commented Mar 18, 2019

Short update: This issue will only be fixed in Sonarqube 7.6+.

@Kovshar-OS
Copy link

Hi, Reamer! What about SonarQube 7.0 ?

@Reamer
Copy link
Member

Reamer commented Mar 26, 2019

Sonarqube 7.0 is a release, which is neither supported from sonarqube nor from this plugin. This plugin tries to support the latest LTS (6.7.x) and the newest version of sonarqube (at the moment 7.7). Because Sonarqube 7.6 has a major change in plugin API (see https://docs.sonarqube.org/display/DEV/API+Changes), i had to rewrite the whole plugin workflow.
If someone has sonarqube 7.6 running, they can test the current rewrite. (https://github.com/stevespringett/dependency-check-sonar-plugin/pull/99/files)

@Reamer
Copy link
Member

Reamer commented Apr 21, 2019

Hi,
please checkout new version of this plugin.
1.1.4 for Sonarqube 6.7.5 and above
1.2.3 for Sonarqube 7.6 and above

@Reamer
Copy link
Member

Reamer commented May 7, 2019

Should be fixed with actual version of this plugin. This plugin looks for a pom.xml or gradle.build in your project to link issue against this files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants