The security analyzer detects more vulnerabilities by making a clear distinction between Sanitizers and Validators

Clean Code Sonar Updates
,
1

Hello SonarCloud users,

The SAST engine for Java, C#, PHP, Python, JavaScript and TypeScript was improved to discard less True-Positive issues from being reported.

Let’s take an example to illustrate the change which introduces 2 concepts: Sanitizer and Validator.

1  <?php
2 
3  $conn = mysqli_connect('best_server', 'nicest_user', 'safest_password');
4 
5  $tainted = $_GET['id'];
6  $sanitized = mysqli_real_escape_string($tainted); // "mysqli_real_escape_string" is a Sanitizer
7  mysqli_query($conn, "SELECT * FROM users WHERE name = '" . $sanitized . "'"); // OK: no issue raised as expected
8  mysqli_query($conn, "SELECT * FROM users WHERE name = '" . $tainted . "'"); // Vulnerable, but was an False-Negative in the past :(
9 
10  if (is_int($tainted)) { // "is_int" is a Validator
11    mysqli_query($conn, "SELECT * FROM users WHERE name = '" . $tainted . "'"); // OK
12  }

In this code:

  • mysqli_real_escape_string is considered as a Sanitizer for SQL Injection vulnerabilities because it return a safe version of the argument which can then be used in a concatenated SQL query.
  • is_int is considered as a Validator because it checks if its argument is having some characteristics and then return true or false.

Previously, the SAST engine was not making any distinction between the two concepts and this was the root cause of issues not being raised because the engine was wrongly considering that some variables were sanitized whereas it was not the case.

In the example, no issue was raised on line 8 because the SAST engine was over approximating the behavior of mysqli_real_escape_string and finally was considering that $sanitized and $tainted were safe.

This is behind us, so don’t be surprised if you see some new vulnerabilities raised on your old code, this is certainly because of this change.

Enjoy!
Alex

2 Likes