Rules to detect missing Java input validation

SonarQube
,
#1

Hi
I am very new to the Sonar universe. I would like to detect missing input validation in Java functions, e.g. as described in OWASP Top Ten Proactive Controls 2018, section C5 (OWASP Top Ten Proactive Controls 2018 | C5: Validate All Inputs | OWASP Foundation).

### BAD CODE (SPRING BOOT)
...

public class Buy {
  private Integer price;
  private Integer quantity;

}

@RestController
class BuyController {
  @PostMapping("/buy")
  public void Buy(@RequestBody Buy buy){
    System.out.println(buy);
  }
}
...

The above code should trigger an alarm. The below code should not (as it has rudimentary input validation):

### SLIGHTLY BETTER CODE (SPRING BOOT)
...
package javax.validation.constraints;

public class Buy {
  @PositiveOrZero
  private Integer price;
  @Positive
  private Integer quantity;

}

@RestController
class BuyController {
  @PostMapping("/buy")
  public void Buy(@Valid @RequestBody Buy buy){
    System.out.println(buy);
  }
}
...

Is this solved in full or part by any existing rule(s)?

SonarQube Enterprise Edition Version 9.4 (build 54424).

Thanks in advance and best regards,
Andreas

#4

Hey @andfaa,

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.

Best,
Karim.

#5

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?

[1] NVD - CVE-2014-4962
[2] NVD - CVE-2020-11007

Best regards,
Andreas

#6

Hey @andfaa,

thank you for the examples. Fair and valid point with unchecked numbers also having the potential of leading to unwanted behavior.

Is there perhaps something we have to enable manually?

No, the rules detecting injection flaws are active by default.

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.

Best regards,
Karim.

1 Like
#8

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.

1 Like