Add Plugin for Security Analysis Tool "Xanitizer" to Update Center

[Norman Wenzel]

Dear SonarQube team,

the plugin for the security analysis tool "Xanitizer" is already on the list of referenced plugins.

Now we have finally made it open source (https://github.com/RIGS-IT/sonar-xanitizer). Could you please add it to the Update Center?

The plugin does not run a security analysis by itself. Instead, it parses the XML report generated by Xanitizer and creates SonarQube issues based on the security vulnerabilities detected by Xanitizer.

If there are any additional requirements please let us know.

Many regards,

Norman Wenzel

RIGS IT GmbH

[Norman Wenzel]

Attached you will find the example Webgoat project provided with the Xanitizer installation together with a sample report (webgoat-Findings-List.xml), so that everybody can test the plugin without using Xanitizer. Thanks to Ann for the proposal

---

sample.zip

[G. Ann Campbell]

Hi Norman,

I've got some feedback for you in two categories:

FYI

* Your property name is "xanitizer.xmlReportFile". By convention, property names start with "sonar.".

* Consider providing a default value for this property, e.g. target/reports/xanitizer.xml

* There's no need to provide a Xanitizer profile, altho this is your prerogative. Because the rules are already grouped by Repository, they are easily findable in the Rules search for addition to the user's own profiles.

* You've named your "JAVA Xanitizer", but profiles are automatically displayed with the relevant language, e.g. 

* https://jira.sonarsource.com/browse/SONAR-7915

Please Fix

* Your docs say to configure the report location via the UI. My first assumption was that this was simply a crossed wire, so I configured it in the sonar-project.properties instead. And the report was not picked up. 

* When I ran an analysis with no Xanitizer rules enabled, I saw this in the log:

ERROR - No Xanitizer rule is set active in the used quality profile. Skipping analysis.

It should be a WARN rather than an ERROR, and it should appear IFF a xanitizer report is provided.

* When I enable Xanitizer rules in the Sonar way profile, nothing seems to happen. At least, the Xanitizer widget is empty after analysis.

* When I use the provided Xanitizer profile, I get hundreds of lines like the following, and then the analysis ends in an error:

14:32:30.465 INFO  - Xanitizer: Skipping finding 10: SpecialMethodCall:JavaServletFindAndInclude
14:32:30.465 INFO  - Xanitizer: Skipping finding 11: SpecialMethodCall:JavaServletFindAndInclude
14:32:30.465 INFO  - Xanitizer: Skipping finding 12: SpecialMethodCall:JavaServletFindAndInclude
14:32:30.465 INFO  - Xanitizer: Skipping finding 13: SpecialMethodCall:JavaServletFindAndInclude
14:32:30.466 INFO  - Xanitizer: Skipping finding 14: SpecialMethodCall:JavaServletFindAndInclude
14:32:30.466 INFO  - Xanitizer: Skipping finding 15: SpecialMethodCall:JavaServletFindAndInclude

And here's the error

14:38:26.403 INFO  - Processing Xanitizer analysis results of 2016-07-01 10:42:25; findings: 912
INFO: ------------------------------------------------------------------------
INFO: EXECUTION FAILURE
INFO: ------------------------------------------------------------------------
Total time: 11.796s
Final Memory: 20M/634M
INFO: ------------------------------------------------------------------------
ERROR: Error during Sonar runner execution
org.sonar.runner.impl.RunnerException: Unable to execute Sonar
at org.sonar.runner.impl.BatchLauncher$1.delegateExecution(BatchLauncher.java:91)
at org.sonar.runner.impl.BatchLauncher$1.run(BatchLauncher.java:75)
...
Caused by: java.lang.NoSuchMethodError: org.sonar.plugins.java.api.JavaResourceLocator.findResourceByClassName(Ljava/lang/String;)Lorg/sonar/api/resources/Resource;
at com.rigsit.xanitizer.sqplugin.XanitizerSensor.mkInputFileOrNull(XanitizerSensor.java:417)
at com.rigsit.xanitizer.sqplugin.XanitizerSensor.generateTaintPathIssue(XanitizerSensor.java:302)
at com.rigsit.xanitizer.sqplugin.XanitizerSensor.generateIssuesForFinding(XanitizerSensor.java:190)
at com.rigsit.xanitizer.sqplugin.XanitizerSensor.createMeasures(XanitizerSensor.java:161)
at com.rigsit.xanitizer.sqplugin.XanitizerSensor.analyse(XanitizerSensor.java:144)

Note that I'm testing with 6.0-RC1.

* In looking more closely at your rules, I see that one of them is "Xanitizer Findbugs Finding". There is already a FindBugs plugin that will report issues raised by FindBugs far more granularly than this. It is a disservice to users to report in one conglomerate category the results of the nearly 450 FindBugs rules which are available granularly.

In fact, based on this rule and the "Xanitizer OWASP Dependency Check Findings" rule, it looks like Xanitizer is some sort of aggregation platform, and you're trying to further aggregate its combined findings into SonarQube? SonarQube is not (any longer) an aggregation platform. 

Based on this and how highly generic your other four rules appear to be, it seems to me right now that this plugin is simply not compatible with the SonarQube ethos. 

Ann

[Norman Wenzel]

Hello Ann,

thank you for your feedback, we will fix these issues as soon as possible.

Am Donnerstag, 28. Juli 2016 21:07:26 UTC+2 schrieb G. Ann Campbell:

In looking more closely at your rules, I see that one of them is "Xanitizer Findbugs Finding". There is already a FindBugs plugin that will report issues raised by FindBugs far more granularly than this. It is a disservice to users to report in one conglomerate category the results of the nearly 450 FindBugs rules which are available granularly.

In fact, based on this rule and the "Xanitizer OWASP Dependency Check Findings" rule, it looks like Xanitizer is some sort of aggregation platform, and you're trying to further aggregate its combined findings into SonarQube? SonarQube is not (any longer) an aggregation platform. 

The idea of the plugin is to see the results of the Xanitizer security analysis in SonarQube. Xanitizer (the main tool, not the plugin) optionally integrates OWASP dependency check and the security relevant FindBugs and FindSecBugs rules so that people don't have to use several tools. They can review these findings, priorize them and so on and than the result of this work should be visible in SonarQube.

It would be quite strange if I have for example 100 findings, export them, but the result would be 65, because "there already are OWASP and FindBugs rules available", together with the lost review effort.

I agree with you that it doesn't make sense to active both, the "normal" FindBugs and DependencyCheck rules AND the Xanitizer version of it. Thats the reason why you can simply switch it of. It is OPTIONAL. But if I WANT to use the Xanitizer rules version, that integrates my already reviewed results of these checks, why shouldn't I do this?

If I see that there are redundancies with other rules, well, then I will switch off the one that I am not interested in. But why don't have the choice?

We have exatly the same problem in the Xanitizer main tool. There are many redundancies between our findings and the FindBugs findings. But we expect the users to be smart enough to choose the information they are interested in.

 

Based on this and how highly generic your other four rules appear to be, it seems to me right now that this plugin is simply not compatible with the SonarQube ethos. 

What do you mean with generic? The reason  for just providing 4 rules is to keep it as simple as possible. Xanitizer analyzes more than 50 problem types at the moment (Command Injection, SQL Injection, several kinds of CrossSiteScripting, Privacy Leaks, Weak Algorithms and so on). Of course we could provide one rule for each supported problem type. But for what reason? Either I want to see the Xanitizer results in SonarQube or I don't want to see them. Why should I only be interested in SQL Injection? I I AM only be interested in SQL Injection, I would configure the tool this way and will enable only this problem type.

So please let me know why the plugin should not be compatible with the SonarQube ethos ("management of the technical debt and the quality of the code")? Xanitizer has at the moment by far the highest score at the OWASP Benchmark (https://www.owasp.org/index.php/Benchmark#tab=Main) and so I think it's results will be quite valuable in a quality platform like SonarQube.

Sorry for the long post, but I hope you understand my point. 

Norman

[Norman Wenzel]

Dear Ann, I have fixed the minor issues, but two things are strange:

Am Donnerstag, 28. Juli 2016 21:07:26 UTC+2 schrieb G. Ann Campbell:

* You've named your "JAVA Xanitizer", but profiles are automatically displayed with the relevant language, e.g. 

No, the quality profile is just named "Xanitizer: 

RulesProfile.create("Xanitizer", Java.KEY);

and is also displayed without a "Java" prefix (5.6):  

Maybe a bug in 6.0 RC?

 

* Your docs say to configure the report location via the UI. My first assumption was that this was simply a crossed wire, so I configured it in the sonar-project.properties instead. And the report was not picked up. 

For me (5.6 again) everything works fine. Could you please check if it was a typo in the property name?

Otherwise, since the property is read from the Settings object, which should be handling the properties, but is transparent for plugin developers, it would indicate a bug in 6.0. 

 

And additionally, I have a question:

ERROR: Error during Sonar runner execution
org.sonar.runner.impl.RunnerException: Unable to execute Sonar
at org.sonar.runner.impl.BatchLauncher$1.delegateExecution(BatchLauncher.java:91)
at org.sonar.runner.impl.BatchLauncher$1.run(BatchLauncher.java:75)
...
Caused by: java.lang.NoSuchMethodError: org.sonar.plugins.java.api.JavaResourceLocator.findResourceByClassName(Ljava/lang/String;)Lorg/sonar/api/resources/Resource;
at com.rigsit.xanitizer.sqplugin.XanitizerSensor.mkInputFileOrNull(XanitizerSensor.java:417)
at com.rigsit.xanitizer.sqplugin.XanitizerSensor.generateTaintPathIssue(XanitizerSensor.java:302)
at com.rigsit.xanitizer.sqplugin.XanitizerSensor.generateIssuesForFinding(XanitizerSensor.java:190)
at com.rigsit.xanitizer.sqplugin.XanitizerSensor.createMeasures(XanitizerSensor.java:161)
at com.rigsit.xanitizer.sqplugin.XanitizerSensor.analyse(XanitizerSensor.java:144)

Looks like we are not the only ones with this problem. Is there any way to get notice of these API changes (the commit message for the change was "Stop feeding design metric and remove cycle between packages rule" - nothing where I would suspect an API change)? Or is there an other way to get the corresponding resource for a class name?

Regards

Norman

 

[G. Ann Campbell]

Hi Norman,

I'm sorry to disappoint you, but I have no plans to re-test your plugin. SonarQube is not an aggregation platform, and as it is currently constituted, I see no path for adding it to the update center..

Ann

P.S. I failed to mention last time that there is already a separate Dependency Check plugin as well.

---

G. Ann CAMPBELL | SonarSource

Product Owner

http://sonarsource.com

--
You received this message because you are subscribed to a topic in the Google Groups "SonarQube" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sonarqube/mXYKHLh1Luk/unsubscribe.
To unsubscribe from this group and all its topics, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/b11880ac-dee4-4d19-aa30-2394998c0a62%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[Norman Wenzel]

Dear Ann, yes, it is indeed disappointing, especially because you didn't respond to my arguments. You wrote your opinion, mention some bugs (that do not occur in the currently released version) and the "discussion" is over. This is quite sad.

To make it clear: Xanitizer IS NOT an aggregation platform. It is a security analysis tool which runs a security analysis based on static code and data flow analysis. ADDITIONALLY it also makes it possible to run the FindBugs and OWASP Dependency Checks, if the user is interested in them. But this IS NOT the main focus of the tool.

Of course I have to accept your  decision, but I can see no contradiction to the goals of SonarQube.

[Fabrice Bellingard]

Hi Norman,

Let's say that you remove the 2 "misleading" rules (the Findbugs and OWASP Dependency Checks) from your plugin to avoir confusion when users have installed the original SonarQube plugins for those 2 technologies: would this be acceptable from your point of view? If they are not the main focus of the tool, then you might not have big expectations on them and this would clarify (maybe even reinforce) the positioning of your tool inside the SonarQube ecosystem?

Best regards,

Fabrice BELLINGARD | SonarSource SonarQube & SonarLint Product Manager http://sonarsource.com

--
You received this message because you are subscribed to the Google Groups "SonarQube" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sonarqube+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/208f620d-99f3-4724-ad05-700868cf33e9%40googlegroups.com.

[Norman Wenzel]

Dear Fabrice,

as I mentioned above, it could be a litte bit confusing if you have reviewed findings in Xanitizer and they will vanish in SonarQube. But if you think that the potential redundancies are more confusing, then of course this is acceptable for us.

A little off topic:

Do you have any clou how to handle the NoSuchMethodError reported by Ann? Since it results from changes in the Java Plugin and not is independent of the SonarQube version, what is a good strategy for a bugfix release? Or should we use java.lang.Object in this case and check it with instanceof?

[G. Ann Campbell]

@Norman & @Fabrice,

I would still like to see the other 4 rules differentiated. @Norman, you said earlier that they represent about 50 granular rules. If that's the case, then your plugin should have 50 granular rules.

Ann

---

G. Ann CAMPBELL | SonarSource

Product Owner

http://sonarsource.com

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/75514c30-4786-4eda-aebe-d389d706c827%40googlegroups.com.

[Norman Wenzel]

Dear Ann,

why do you think that this is necessary? 

In my opinion it will make the usage of the plugin more complicated. The main use case is that the Xanitizer issues are visible in SonarQube. Why should it be helpful to activate just for example Command Injection and ignore SQL Injection? Providing a single rule per problem type would make sense if it is used to configure something (like a field name pattern and so on), but the rules are just used to switch the issue creation on and off.

If you persist on it, then we will of course change it, but I just can't see your point. But maybe my understanding of the rules concept is wrong?

[G. Ann Campbell]

Hi Norman,

The point is to give the user a good experience.

For example, if we wanted we could probably boil all the rules in our repository down to about 4:

• Don't do bad things
• Don't write insecure code
• Don't do silly things
• Don't write useless code
• Don't write code that's hard to read

But we don't because users won't always understand what's "bad" about a given piece of code or why it's "insecure."

We write more granular rules so that users can better understand why a piece of code is bad or dangerous, and learn to write better code. 

Also, having granular rules allows us to put the "lesson" in the rule description rather than - as it appears from your xml report - in the message. Messages should be short, directive, and to the point. They should at best tell the user what remediation to take, not describe why it needs to be taken. - That's bad UX. Imagine that I have 14 issues from the same micro-rule of yours. To see them I have to read the sermon 14 times, instead of reading it once (if I don't already know it) when I click to see the rule description.

Feel free to browse through our rules to get an idea of how this works.

Ann

---

G. Ann CAMPBELL | SonarSource

Product Owner

http://sonarsource.com

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/59e1eeba-b66c-4e8b-9ec6-0a49b97936c1%40googlegroups.com.

[Norman Wenzel]

Hello Ann,

thanks for the explanation, that was convincing and also makes it clear why providing the FindBugs and DependencyCheck rules twice is a bad idea. I simple had a different understanding of the rules concept.

We will re-work the plugin.

[Fabrice Bellingard]

On Tue, Aug 2, 2016 at 10:55 AM, Norman Wenzel <normanwe...@gmail.com> wrote:

Dear Fabrice,

as I mentioned above, it could be a litte bit confusing if you have reviewed findings in Xanitizer and they will vanish in SonarQube. But if you think that the potential redundancies are more confusing, then of course this is acceptable for us.

Great!

 

A little off topic:

Do you have any clou how to handle the NoSuchMethodError reported by Ann? Since it results from changes in the Java Plugin and not is independent of the SonarQube version, what is a good strategy for a bugfix release? Or should we use java.lang.Object in this case and check it with instanceof?

Can you please open new thread for those questions so that we don't mix topics? Thanks!

 

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/75514c30-4786-4eda-aebe-d389d706c827%40googlegroups.com.

[Norman Wenzel]

Dear Ann, dear Fabrice,

I have re-worked the plugin. The FindBugs and OWASP Dependency Check rules are removed now and the generic rules are replaced by a single rule for each problem type together with its description. Additionally, the other issues reported by Ann are fixed.

I would be glad if you could re-review the plugin now.

Thanks

Norman

[G. Ann Campbell]

Hi Norman,

I've taken a look at the new version of the plugin.

* Individual rules: check

* Default report path: check

* Parameter naming convention: check

* Peport file location read from sonar-project.properties: check

* Analysis without exceptions: check

But I'm still seeing:

* A log warning when no Xanitizer rules are enabled: 14:56:14.649 WARN  - Xanitizer XML report file '/path/to/project/Xanitizer-Findings-List.xml' not found. Skipping analysis.

New stuff:

* The vast, vast majority of issues raised in the sample project (Webgoat) you provided were raised at project level. This is not acceptable, and what makes it doubly frustrating is that you appear in the messages to cite the files the issues should be attached to. SonarQube is about code quality. I.e. you must attach issues to the code. 

I suspect this is because you associated all the rules with Java (pretty smart choice, since that plugin is included by default in a new install) and many of the files you're raising issues on are not .java files. The SonarQube Analyzer for Java team faced this same problem with some rules they added last year for certain xml files. You might want to take a closer look at how they tackled the issue.

* Issue messages should be imperative messages telling users how to fix the issue. What I'm looking at starts with a restatement of the rule title ("Log Injection") followed by what appears to be diagnostic information and the path to the relevant file. You should never need to include a file path in an issue message (see above) because the issue is attached to the file in question. In short, this is poor UX.

Further, the paths in these messages seem to have nothing to do with my project. I'm looking at an issue with this in the message:  /${INSTALL_DIR}/examples/webgoat/lessons/CrossSiteScripting/Login.jsp:26 

Assuming that ${INSTALL_DIR} was the project root (because everything in SonarQube starts from there) I looked for the examples directory because I wanted to see the issue in the file (and the issue's not attached to the file!!). But it doesn't exist. It took me a couple of minutes to realize that /${INSTALL_DIR}/examples/webgoat/ is completely irrelevant to my analysis, which started from within the webgoat directory itself.

* I've finally found the file that corresponds to 

Password in config file (Xanitizer finding ID 3634) - /${INSTALL_DIR}/examples/webgoat/lessons/CrossSiteScripting/Login.jsp:26 

And I see that line 26 of Login.jsp is this: 

    <input name="password" type="password" size="10" maxlength="8" />

And all I can say is... REALLY!?!

* So then I decided to take a look at where you actually raise issues in files. I ended up at Screen.java, which has 46 issues tagged "xanitizer". All 46 are raised on this line:

		out.print(getContent());

That breaks down to 24 instances of "XSS Stored", 11 instances of "XSS Reflected", and 11 instances of "Privacy Leak".  

I'm going to just walk away with my professionalism at this point.

Ann

[Norman Wenzel]

Hi Ann,

Am Dienstag, 9. August 2016 21:39:28 UTC+2 schrieb G. Ann Campbell:

But I'm still seeing:

* A log warning when no Xanitizer rules are enabled: 14:56:14.649 WARN  - Xanitizer XML report file '/path/to/project/Xanitizer-Findings-List.xml' not found. Skipping analysis.

No, not still. The last time you complained, that the tool shows a warning when the rules are not active, even if no report file was provided. This time it is the other way around. ;)

But since this could never happen again (because of the given default of the property), we could of course switch the logic of the check again.

 

New stuff:

* The vast, vast majority of issues raised in the sample project (Webgoat) you provided were raised at project level. This is not acceptable, and what makes it doubly frustrating is that you appear in the messages to cite the files the issues should be attached to. SonarQube is about code quality. I.e. you must attach issues to the code.

I suspect this is because you associated all the rules with Java (pretty smart choice, since that plugin is included by default in a new install) and many of the files you're raising issues on are not .java files. The SonarQube Analyzer for Java team faced this same problem with some rules they added last year for certain xml files. You might want to take a closer look at how they tackled the issue.

The problem is, that many issues occur in code that is generated by Xanitizer itself (to simulate the behaviour of Web Frameworks) and that these files do not have a representation in the "real" code of the project. We thought it would be better to create this issues on project level together with their file path than to simply skip the issue, so that the user knows at least that there IS a problem, even if it could not be displayed in SonarQube.

We could change this and only generate issues that could be matched to files this is important to you. But I hope you can see my point.

For all other (non-Java) resources I will check the Java plugin.

 

* Issue messages should be imperative messages telling users how to fix the issue. What I'm looking at starts with a restatement of the rule title ("Log Injection") followed by what appears to be diagnostic information and the path to the relevant file. You should never need to include a file path in an issue message (see above) because the issue is attached to the file in question. In short, this is poor UX.

The reason for adding the path you can find above.

Writing imperative messages for a security vulnerability that do always match would be nearly impossible. Sure, some things are simple, but in general, it is not. And a message like "Check this variable for forbidden characters" is not helpful and aditionally, in most cases not done at the location of the issue. It is simply different to coding style rules and so on.

Further, the paths in these messages seem to have nothing to do with my project.

 

Not a surprise, since they are parsed from the report, which was generated on MY machine. ;)

For Java classes, this is not a problem, because they are identified by their class name. But for other resources there is no chance. 

 

* I've finally found the file that corresponds to 

Password in config file (Xanitizer finding ID 3634) - /${INSTALL_DIR}/examples/webgoat/lessons/CrossSiteScripting/Login.jsp:26 

And I see that line 26 of Login.jsp is this: 

    <input name="password" type="password" size="10" maxlength="8" />

And all I can say is... REALLY!?!

A false-positive that is matched by a reg-ex. Could happen, don't you think so?

 

* So then I decided to take a look at where you actually raise issues in files. I ended up at Screen.java, which has 46 issues tagged "xanitizer". All 46 are raised on this line:

		out.print(getContent());

That breaks down to 24 instances of "XSS Stored", 11 instances of "XSS Reflected", and 11 instances of "Privacy Leak".  

I'm going to just walk away with my professionalism at this point.

Xanitizer is doing a taint path analysis and a security vulnerability is a path of the tainted through the system. But representing paths in SonarQube is not possible. So we had to decide where to raise the issue. Should it be the starting point of the path (i.e. the taint source) or the ending point (i.e. the taint sink). We decided for the taint sink, because this is the location where the tainted data might cause harm.

So of course there could be several issues at the same location - these are paths with the same end but different starting points. In the prior version, the message additionally contained the location of the starting point, so that the issues can be distinguished, but we have removed that (no file paths in the message...).

[G. Ann Campbell]

Gah! I just realized I didn't "Reply All"

My apologies to everyone I've ever chastised for this

---

G. Ann CAMPBELL | SonarSource

Product Owner

http://sonarsource.com

---------- Forwarded message ----------
From: G. Ann Campbell <ann.ca...@sonarsource.com>
Date: Wed, Aug 10, 2016 at 5:25 PM
Subject: Re: Add Plugin for Security Analysis Tool "Xanitizer" to Update Center
To: Norman Wenzel <normanwe...@gmail.com>

On Wed, Aug 10, 2016 at 8:45 AM, Norman Wenzel <normanwe...@gmail.com> wrote:

No, not still. The last time you complained, that the tool shows a warning when the rules are not active, even if no report file was provided. This time it is the other way around. ;)

But since this could never happen again (because of the given default of the property), we could of course switch the logic of the check again.

Nevermind

 

The problem is, that many issues occur in code that is generated by Xanitizer itself (to simulate the behaviour of Web Frameworks) and that these files do not have a representation in the "real" code of the project. We thought it would be better to create this issues on project level together with their file path than to simply skip the issue, so that the user knows at least that there IS a problem, even if it could not be displayed in SonarQube.

We could change this and only generate issues that could be matched to files this is important to you. But I hope you can see my point.

For all other (non-Java) resources I will check the Java plugin.

From the Deploying to Update Center docs (emphasis added):

Last but not least: your plugin must be aligned with the goal of the SonarQube platform: management of the technical debt and the quality of the code

• To be more precise: every feature of SonarQube is tied to the code, so if your plugin provides data that can't be attached to a source or a test file, then there are chances that your plugin won't be accepted in the Update Center

So, yes. It is important. I hope you can see my point.

 

Writing imperative messages for a security vulnerability that do always match would be nearly impossible. Sure, some things are simple, but in general, it is not. And a message like "Check this variable for forbidden characters" is not helpful and aditionally, in most cases not done at the location of the issue. It is simply different to coding style rules and so on.

Uhm... Not entirely sure I'm following you. Why isn't the SonarQube issue raised "at the location of the issue"? 

And even if you think "Check this variable for forbidden characters" is unhelpful (not sure I agree), you can fall back on a simple statement of helpful fact: "This variable may contain forbidden characters and should not be logged." Or whatever.

 

Further, the paths in these messages seem to have nothing to do with my project.

 

Not a surprise, since they are parsed from the report, which was generated on MY machine. ;)

This goes back to the fact that you shouldn't be showing me paths in issue messages; you should be finding the relevant files and attaching the issues to them.

 

For Java classes, this is not a problem, because they are identified by their class name. But for other resources there is no chance. 

Not sure I agree. I was eventually able to locate lessons/CrossSiteScripting/Login.jsp. Granted, I was able to bring human intelligence to bear on the problem, but with a set of preconditions (such as requiring that Xanitizer be initiated from the project root so that the report paths will make sense in a SonarQube context) I think this is very doable. 

  

* I've finally found the file that corresponds to 

Password in config file (Xanitizer finding ID 3634) - /${INSTALL_DIR}/examples/webgoat/lessons/CrossSiteScripting/Login.jsp:26 

And I see that line 26 of Login.jsp is this: 

    <input name="password" type="password" size="10" maxlength="8" />

And all I can say is... REALLY!?!

A false-positive that is matched by a reg-ex. Could happen, don't you think so?

I'd have immediately given you the benefit of the doubt on this one except that it's hard for me to conceive of a .jsp file as a "config file". .properties, yes. .txt, sure. .jsp? No. The fact that you raised this issue on an HTML input named "password" seems egregious. But maybe that's just mee.

  

Xanitizer is doing a taint path analysis and a security vulnerability is a path of the tainted through the system. But representing paths in SonarQube is not possible.

Not true. At least not within a single file. But that's beside the point

 

So we had to decide where to raise the issue. Should it be the starting point of the path (i.e. the taint source) or the ending point (i.e. the taint sink). We decided for the taint sink, because this is the location where the tainted data might cause harm.

So of course there could be several issues at the same location - these are paths with the same end but different starting points. In the prior version, the message additionally contained the location of the starting point, so that the issues can be distinguished, but we have removed that (no file paths in the message...).

I suggest you rethink this. Giving me 24 instances of the same issue with essentially the same message (except for a "Xanitizer finding ID", which I shouldn't have to care about in this context) on the same line is NOT good UX. How am I supposed to distinguish among these? How am I supposed to know what to do to fix the first one, let alone the 12th? You've got to either boil this down to one issue per type per location, or find a different way of going about this.

And now that I've gotten past the shock, and looked at another file, you need to take a look at where you attach issues in general:

BTW, some issue locations do look good. I'm looking at a SQL injection issue, and it's well located.

But others are just too aggressive. I'm looking at two "IO Stream Resource Leak" issues you've raised on a "catch" statement. You already raised issues on the creation of the unclosed resources (which IMO is the best place for it), so raising issues on the "catch" is 1) redundant, and 2) just plain wrong. It's not the catch that leaks anything. 

And huh?

Also this one:

If in the rule description you told me that all close() calls should happen in finally blocks, then I'd go along with it, but to a naive user, this is just a false positive.

At this point, I'm really wondering if you ran this report import on your side and found the results 1) clear, 2) helpful, 3) actionable. Really. Imagine that you're not the Xanitizer dev, but a mid-level coder who's seeing this stuff for the first time. Would you know what to do?

Ann

[Norman Wenzel]

I already wondered, why I got a mail, but couldn't find anything in the thread ;)

Thank you for your feedback! Even if we sometimes have different opinions, I appreciate it very much!

To make it short, I can see your points, even if some of them have nothing to do with the plugin itself, because they simply result from the output of the Xanitizer main tool. In the tool, the context is clearer, but still it can be confusing I have to admit. We have reworked the XML output, polished some of the reported locations and added (I hope so) clear messages what to do.

The last problem that I am facing now are the XML resources and rules. If the XML plugin is available, everything works fine. But if not, the XML files are not contained in the FileSystem and the XML rules can not be enabled. Is this the designed/desired behaviour or is there are way to check these files even if the language plugin is not contained?

[G. Ann Campbell]

Hi Norman

On Fri, Aug 19, 2016 at 1:39 PM, Norman Wenzel <normanwe...@gmail.com> wrote:

I already wondered, why I got a mail, but couldn't find anything in the thread ;)

Thank you for your feedback! Even if we sometimes have different opinions, I appreciate it very much!

Thanks for not being a grouch! 

:-)

 

To make it short, I can see your points, even if some of them have nothing to do with the plugin itself, because they simply result from the output of the Xanitizer main tool.

I'd like to gently disagree with this. IMO, it's the job of the plugin to filter the tool's output for the best user consumption in SonarQube. This is a trivial example, but I believe that when you run an analysis that includes even a single FindBugs rule, when FindBugs is invoked it runs all its rules. It is the job of the FindBugs plugin to filter the FindBugs output to present only those issues the user needs to see. (I.e. the ones that correspond to rules included in the profile.)

I'm not sure I'm remembering correctly, but it seems that if I open a Closeable, your tool flags where it's opened and each spot in the code where control flow loses access to the resource to be able to close it.

Assuming you can stitch together in the plugin that all of those issues go together, I'd raise the one on the open, and filter out the rest. Or even better, feed them into the primary issue (the one raised when the resource is opened) as secondary locations. THAT would be a great service to the user!

 

<snip>

The last problem that I am facing now are the XML resources and rules. If the XML plugin is available, everything works fine. But if not, the XML files are not contained in the FileSystem and the XML rules can not be enabled. Is this the designed/desired behaviour or is there are way to check these files even if the language plugin is not contained?

We had the same problem in the Java plugin when we wanted to check Java-related, non-.java files. Here's how we addressed it. It would seem perfectly reasonable to me to have similar advice in your docs. In fact, the first time I tested your plugin, I turned on "Import unknown files" to see if the results would get any better.

Notice I didn't say "here's how we solved it" because we don't consider this a solution but a workaround. We have it on our "soon" list to remove restrictions on which plugins can see which files. But don't hold your breath. :-)

HTH,

Ann

---

G. Ann CAMPBELL | SonarSource

Product Owner

http://sonarsource.com 

[Norman Wenzel]

Am Freitag, 19. August 2016 20:17:14 UTC+2 schrieb G. Ann Campbell:

I'm not sure I'm remembering correctly, but it seems that if I open a Closeable, your tool flags where it's opened and each spot in the code where control flow loses access to the resource to be able to close it.

Assuming you can stitch together in the plugin that all of those issues go together, I'd raise the one on the open, and filter out the rest. Or even better, feed them into the primary issue (the one raised when the resource is opened) as secondary locations. THAT would be a great service to the user!

Yeah, thats exactly what we did now.

 

 

We had the same problem in the Java plugin when we wanted to check Java-related, non-.java files. Here's how we addressed it. It would seem perfectly reasonable to me to have similar advice in your docs. In fact, the first time I tested your plugin, I turned on "Import unknown files" to see if the results would get any better.

Thanks, I think this workaround is good enough.

Attached you can find a new version of the report (to handle the problems raised by the tool itself) and a new version of the plugin is released on GitHub. It would be nice if you could give it another chance (and maybe will be pleased with the result this time ;)  ).

[G. Ann Campbell]

Hi Norman,

This review iteration was definitely a much more pleasant experience! I've added the plugin to the update center. :-D

As a reminder, future plugin releases will need to go through a Request For Feedback (RFF) period.

Also, FYI:

* there seems to be a length limit on secondary messages

* It's odd to me that your plugin project has a sonar-project.properties file in addition to the pom. (But whatever makes you happy.)

* When I turned on unknown file import I got the same number of issues as without it. Was that expected?

Ann

---

G. Ann CAMPBELL | SonarSource

Product Owner

http://sonarsource.com

--

You received this message because you are subscribed to a topic in the Google Groups "SonarQube" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sonarqube/mXYKHLh1Luk/unsubscribe.

To unsubscribe from this group and all its topics, send an email to sonarqube+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/f113677b-1e78-4919-8f3f-55fd4af7be9a%40googlegroups.com.

[Norman Wenzel]

Am Mittwoch, 24. August 2016 16:21:35 UTC+2 schrieb G. Ann Campbell:

This review iteration was definitely a much more pleasant experience! I've added the plugin to the update center. :-D

Thanks, Ann, especially for your patience. All good things are worth waiting for ;)

 

As a reminder, future plugin releases will need to go through a Request For Feedback (RFF) period.

Yeah, I know that. 

 

* When I turned on unknown file import I got the same number of issues as without it. Was that expected?

Hm, did you have the XML plugin installed?

Alltogether, there should be 180 issues, with 5 of them in non-Java files (4 in WEB-INF/web.xml and 1 in WEB-INF/server-config.wsdd).

Do you get only 175 issues or 180 issues, even without unknown files?

[G. Ann Campbell]

On Wednesday, 24 August 2016 11:07:49 UTC-4, Norman Wenzel wrote:

Hm, did you have the XML plugin installed?

No. That's why I turned on 'Import Unknown Files'

 

Alltogether, there should be 180 issues, with 5 of them in non-Java files (4 in WEB-INF/web.xml and 1 in WEB-INF/server-config.wsdd).

Do you get only 175 issues or 180 issues, even without unknown files?

I got 175 both with and without unknown files.

Ann 

[Norman Wenzel]

Could you please check the sonar-project.properties file of the sample project?

In the uploaded version, sonar.sources is set to 'WEB-INF/classes'. To additionally collect the non-Java files it has to be set to '.' .

[G. Ann Campbell]

Yup. That was it. When I change sonar.sources, I see all the issues.

Ann

---

G. Ann CAMPBELL | SonarSource

Product Owner

http://sonarsource.com

On Wed, Aug 24, 2016 at 11:25 AM, Norman Wenzel <normanwe...@gmail.com> wrote:

Could you please check the sonar-project.properties file of the sample project?

In the uploaded version, sonar.sources is set to 'WEB-INF/classes'. To additionally collect the non-Java files it has to be set to '.' .

--
You received this message because you are subscribed to a topic in the Google Groups "SonarQube" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sonarqube/mXYKHLh1Luk/unsubscribe.
To unsubscribe from this group and all its topics, send an email to sonarqube+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/c2bc1a85-82bd-4b31-ac35-cc6136bc490c%40googlegroups.com.

[Norman Wenzel]

Dear Ann,

I think I have done something really stupid...

I have updated the pom file to add the information in the manifest that are used in the update center and then rebuild the plugin.

The idea was to simply replace the jar file of the release, but now I am getting "Error while downloading plugin 'xanitizer' with version '1.3.2'. No compatible plugin found." when I try to install the plugin. Same message occurs if I reuse the original jar file.  

What can be done? Please help!

Norman

[G. Ann Campbell]

Hi Norman,

Can I guess that you've reverted the jar by now? Anyway, download works for me.

Strictly speaking, though, you shouldn't be making changes to a released version. Those changes should be in a new version. (A point release would be fine here.)

Ann

---

G. Ann CAMPBELL | SonarSource

Product Owner

http://sonarsource.com

--

You received this message because you are subscribed to a topic in the Google Groups "SonarQube" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sonarqube/mXYKHLh1Luk/unsubscribe.
To unsubscribe from this group and all its topics, send an email to sonarqube+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/df1d5a56-6460-4c23-8729-4482d8c42d2f%40googlegroups.com.

[Norman Wenzel]

Yes, the JAR is reverted to the original version and a new release 1.3.3 is build containing the updated information for the manifest in its pom file.

Well, I just wanted to avoid the release process, because nothing else has changed, but okay, I have learned my lesson now ;)

Could you simply replace the version in the Update center or should I follow the steps described in "Announcing new releases"?

Am Mittwoch, 14. September 2016 10:44:48 UTC+2 schrieb G. Ann Campbell:

Hi Norman,

Can I guess that you've reverted the jar by now? Anyway, download works for me.

Strictly speaking, though, you shouldn't be making changes to a released version. Those changes should be in a new version. (A point release would be fine here.)

Ann

---

G. Ann CAMPBELL | SonarSource

Product Owner

http://sonarsource.com

On Tue, Sep 13, 2016 at 6:52 PM, Norman Wenzel <normanwe...@gmail.com> wrote:

Dear Ann,

I think I have done something really stupid...

I have updated the pom file to add the information in the manifest that are used in the update center and then rebuild the plugin.

The idea was to simply replace the jar file of the release, but now I am getting "Error while downloading plugin 'xanitizer' with version '1.3.2'. No compatible plugin found." when I try to install the plugin. Same message occurs if I reuse the original jar file.  

What can be done? Please help!

Norman

--
You received this message because you are subscribed to a topic in the Google Groups "SonarQube" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/sonarqube/mXYKHLh1Luk/unsubscribe.

To unsubscribe from this group and all its topics, send an email to sonarqube+...@googlegroups.com.

[G. Ann Campbell]

Sorry, but no, I can't. A new release means an RFF (requiest for feedback) period. You should start a new thread saying that you'd like to put out a new release. It should include the list of (non)changes. Typically RFF periods last for 3 days, although that's up to you. 

Then I can add it as a new version in the update center.

I know this may seem like pointless bureaucracy, but making exceptions gets messy fast.

:-/

Ann

---

G. Ann CAMPBELL | SonarSource

Product Owner

http://sonarsource.com

To unsubscribe from this group and all its topics, send an email to sonarqube+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/sonarqube/4c40ec4a-1249-4c71-b2af-ee33da71c9c3%40googlegroups.com.

[Norman Wenzel]

Yes, thats understandable, but exactly this is the reason, why someone could get the idea of simply replacing the jar for a tiny fix... ;)