thank you for your interest in our security rules.
What kind of vulnerability would you expect to be raised in the first sample?
I do not see a vulnerability because:
System.out.println is not really a dangerous method in this context. For example, the argument is not printed in the HTTP response (in such case we could talk about a potential Cross-site Scripting vulnerability)
Even if the argument would be passed to a dangerous method, it would only contain fields of type Integer. Such fields cannot contain a potentially malicious payload.
Our analyzer considers these points and does not raise a vulnerability.
Is this solved in full or part by any existing rule(s)?
If we talk generally about not raising a vulnerability alarm when potentially malicious input was validated/sanitizated, yes, our analyzer does that.
Hi Karim, thank you for your response. I did indeed write generally about missing sanitation of potentially malicious input. There are many examples where programmers in code assuming a positive integer and not actually checking for for it, with catastrophic results when negative numbers are used, e.g. [1,2].
I would expect such issues to be raised in SonarQube under “Security category” / “OWASP Top 10 2021” / “A03:2021 – Injection” or “Security category” / “OWASP Top 10 2017” /“A1:2017-Injection”. Unfortunately, I have yet to find any such example when using SonarQube on our massive Java codebase. Is there perhaps something we have to enable manually?
Regarding the subject of injection flaws and numbers:
Currently, when it comes to injection vulnerabilities, the analyzer does consider data that is a number to be safe. Reasoning:
The analyzer does not raise a potential injection vulnerability alarm solely based on the knowledge that user input was received somewhere without being validated. This could lead to a lot of noise. Maybe the data is sanitized/validated before its usage somewhere else in the code.
When an injection vulnerability is raised (be it an SQL Injection, Cross-Site Scripting, Path Injection, etc…), it is done based on the fact that user input was detected and it is used in a specific operation without being appropriately sanitized/validated in the path from reception to usage.
The kind of operation depicts what kind of vulnerability might get raised (e.g., If the data is used in an SQL query being executed, an SQL Injection vulnerability might get raised)
The specific kind of vulnerability that might get raised depicts what is considered a correct validation/sanitization or not (e.g., encoding HTML tags is generally a good protection against Cross-Site Scripting when the data is output in the HTTP response, but not a good protection against SQL Injection when the data is used in a query).
Deciding at the operation whether only having made sure that the user input is a number should be considered a good protection or not is highly dependent on the context and on the intentions of the developer
Just an example: Maybe the number is used in an SQL query that updates a quantity. In this case, both negative numbers to decrease the quantity and positive numbers to increase the quantity might be allowed.
The intentions of the developer are hard (more like impossible) to reliably detect by a code analyzer. Some heuristics based on the context can be applied, but, from experience, we found that considering user input that is a number to always be safe does lead to much fewer false positive alerts.
Yes, there are some edge cases here, and if we see many real code examples of false negatives (situations that lead to security vulnerabilities, but the analyzer did not raise an alert), we might revisit this decision.
To be more precise here: they are active by default as long as the built-in quality profile (called Sonar way) is used. If a custom profile is in use, one needs to check if these rules are activated.