False positives and false negatives with rule S1313

[goroll...@gmail.com]

Hi,

the rule S1313 does not match IP addresses but strings consisting of four numbers in [0,255] separated by dots. E.g., 1.2.3.4, or 127.0.0.1
As a consequence even version numbers produce matches if the version number happens to consist of four parts.
The string 255.255.255.255 triggers an issue although it is the IPv4 broadcast address, has a well known meaning and though is exempt from arguments brought forth in https://groups.google.com/d/msg/sonarqube/36EofymqRxI/UBul8bIaAgAJ

Lastly, IPv6 addresses are not detected.

If changes to the rule are not feasible or your opinion regarding the results differ, please consider removing the rule from the standard SonarWay quality profile.

Cheers,
Oliver

[G. Ann Campbell]

Hi Oliver,

This rule has been implemented for four different languages so far. With which are you concerned?

Ann

[goroll...@gmail.com]

Hi Ann!

Sorry, I wasn't aware that tags are not shown in the mobile version. My concerns are about the Java implementation, specifically https://github.com/SonarSource/sonar-java/blob/0d545786c262e5ec0aa4e14f0763ef98ec670c37/java-checks/src/main/java/org/sonar/java/checks/HardcodedIpCheck.java.

Oliver

[Michael Gumowski]

Hello Olivier,

Indeed, IPv6 are not covered by the current implementation of the rule in SonarJava. I consequently created the following ticket to handle them: SONARJAVA-2783

Regarding the False Positives (FPs):

Rule S1313 is checking for "Vulnerabilities" in code, so by definition we expect such rules to have a relatively high rate of FPs. The last thing you want to let through is forgotten hardcoded IP addresses used for tests, for instance. We consider then that the responsibility of validating/invalidating the issues is given to the dev/auditors (security audit?). You will consequently probably be happy to learn that we are currently working on rethinking our approach regarding how vulnerabilities and security hotspots are handled in SonarQube. This is still ongoing work, at a very early stage, and future version of SonarQube will make things a bit clearer (and normally reduce the noise for devs). To follow development of the feature: MMF-1266.

Now, and mainly for the reasons expressed before, the FPs on version numbers which occurs to have the adequate number of groups will stay as side effect of being on hardcoded strings. It's of course quite difficult to identify context in which the value is used, and we are not planning to update the rule to improve this aspect. We are also not going to white list the general broadcast (255.255.255.255), as we prefer to have a rule flagging everything, and let the responsibility to the security auditors to validate/invalidate the findings. Starting a white list of authorized IPs would also lead most probably to a maintenance hell.

Finally, and because this rule is making some noise while it should be considered as a hotspots, we are going to drop it from SonarWay. However, the change will only be visible with next release of SonarJava.

Cheers,

Michael

--
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/620ad29e-dd04-4d9c-a06f-ee67957fcb71%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Michael Gumowski | SonarSource

Software Developer, Language Team
https://www.sonarsource.com