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

FindSecBugs rules are not located in proper Security Category in SonarQube #392

Open
andrew2809 opened this issue Oct 28, 2021 · 7 comments

Comments

@andrew2809
Copy link

I'm not sure if it's a bug or a feature. FindSecBugs contains security rules. They are visible in Security category as Others.
I expected to find them in fine grained category.
I.e.
"Potential JDBC Injection (Spring JDBC)"
https://find-sec-bugs.github.io/bugs.htm#SQL_INJECTION_SPRING_JDBC
should be in OWASP: Top 10 A1-Injection category.

image

@gtoison
Copy link
Contributor

gtoison commented Oct 31, 2021

Hello,
The SpotBugs plugin uses RulesDefinitionXmlLoader from the SonarQube API to load rules from XML files. It looks like this class does not handle OWASP categories (it does not call the addOwaspTop10() method).
Given that RulesDefinitionXmlLoader was deprecated in SonarQube 9 I doubt that SonarSource will improve it at this point.

Eventually the plugin will need to move to a new way of loading rules but that's not an easy refactor

@andrew2809
Copy link
Author

Hi,
Does it mean findbugs security bugs were never reported under 'OWASP Top 10' or were there some api changes recently ?
I am using Community Edition Version 8.9.2.

@gtoison
Copy link
Contributor

gtoison commented Nov 8, 2021

It is possible to link rules definitions with security standards since SQ 7.3 according to: https://jira.sonarsource.com/browse/SONAR-10986
Before that rules had tags and that's what the SpotBugs plugin does, for instance you should find rule findsecbugs:SQL_INJECTION_JDBC under tag owasp-a1 in: https://<YOUR_SQ_SERVER>/sonarqube/coding_rules?tags=owasp-a1

I've looked a bit more in the way rules are created by this plugin and unfortunately there's no easy to add the security categories. I think I'll try asking if there's a way to improve/extends SonarQube's RulesDefinitionXmlLoader

@pethers
Copy link
Contributor

pethers commented Nov 16, 2021

Did a workaround in sonar-cloudformation-plugin https://github.com/Hack23/sonar-cloudformation-plugin/blob/master/src/main/java/com/hack23/sonar/cloudformation/CloudformationRulesDefinition.java , used tags in the xml.

Possible to use newRule.addCwe and newRule.addOwaspTop10 . Not perfect but it works.

@gtoison
Copy link
Contributor

gtoison commented Nov 16, 2021

Ah yes, using the tags through reflection is a good idea.

Then I think that a prerequisite is to upgrade the tags from OWASP 2013 to OWASP 2017 (SonarQube apparently has not updated to OWASP 2021)
I'm not sure why PR #238 was not merged in the end.
@pethers do you want to give a try or shall I look into it?

@pethers
Copy link
Contributor

pethers commented Nov 16, 2021

Found some mappings https://blog.51sec.org/2018/02/owasp-top-10-2010-2013-2017.html

The file https://github.com/spotbugs/sonar-findbugs/blob/master/src/main/resources/org/sonar/plugins/findbugs/rules-findsecbugs.xml only contain owasp-a1, owasp-a3, owasp-a4, owasp-a6.

is findbug sec bugs using 2013 ?

Busy the next few days but will probably have time during the weekend to give a try.

But #238 looks promising so maybe better to look into why is wasn't merged.

@gtoison
Copy link
Contributor

gtoison commented Nov 18, 2021

From what I've seen findbugs-sec is not really categorizing bugs (although some of them have links to the relevant category on the OWASP website)
The SpotBugs plugin uses the findbugs-sec messages.xml and transforms it into XML files using a Groovy script. The OWASP tags are added by the Groovy script

I've also seen that SonarSource plans to migrate to OWASP 2021 in SonarQube 9.4
I'm not sure how we could support OWASP 2017 and 2021 at the same time, depending on the SQ version

gtoison added a commit to gtoison/sonar-findbugs that referenced this issue May 14, 2022
- RulesDefinitionXmlLoader is deprecated and will be dropped in SQ 10
- RulesDefinitionXmlLoader does not populate the OWASP categories and
can't be extended (see spotbugs#392)
- Loading/unloading the plugins dynamically from jar files leads to file
leaks because SpotBugs does not close the classloaders (solves #128)
- The Groovy scripts seem to need some fixes to work with Groovy 4
gtoison added a commit that referenced this issue May 26, 2022
* deps: drop deprecated RulesDefinitionXmlLoader

- RulesDefinitionXmlLoader is deprecated and will be dropped in SQ 10
- RulesDefinitionXmlLoader does not populate the OWASP categories and
can't be extended (see #392)
- Loading/unloading the plugins dynamically from jar files leads to file
leaks because SpotBugs does not close the classloaders (solves #128)
- The Groovy scripts seem to need some fixes to work with Groovy 4

* deps: removed usage of dependencies no longer provided

The sonar-java-plugin is provided by the SonarQube server but not its
transient dependencies
- Removed usage of Guava
- Replaced commons-lang usage by commons-lang3
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

3 participants