PHP SQL Injection False Negative (S3649)

Hey SonarQube community.

I want to report a False Negative. We configured Sonarqube to run all over our PHP code base to check for pieces of code that may contain SQL Injections.

During this analysis, we also did some tests to check the Sonarqube outputs, enabling all PHP vulnerabilities, and created some PHP files that contain a real case SQL Injection vulnerability.

The first vulnerable file to SQL Injection contains the following PHP code, and Sonarqube has not identified any vulnerability, only code smell, as you can see below.

<?php
$p = $_POST['param'];
$sql = 'SELECT name, color, calories FROM fruit ORDER BY ' . $p;
foreach ($conn->query($sql) as $row) {
    print $row['name'] . "\t";
    print $row['color'] . "\t";
    print $row['calories'] . "\n";
}
?>

The second vulnerable code is a blind SQL Injection, as you can see below.

<?php
require_once('../_helpers/strip.php');
// this database contains a table with 2 rows
// This is my first secret (ID = 1)
// This is my second secret (ID = 2)
$db = new SQLite3('test.db');
if (strlen($_GET['id']) < 1) {
  echo 'Usage: ?id=1';
} else {
  $count = $db->querySingle('select count(*) from secrets where id = ' . $_GET['id']);
  if ($count > 0) {
    echo 'Yes!';
  } else {
    echo 'No!';
  }
}
?>

The third vulnerable file uses the same vulnerable code available in your documentation as Noncompliant, and even with this sample code, the Sonarqube was not able to detect any SQL Injection vulnerability, only bugs and code smell, even the bug doesn’t mention anything about SQL Injection as you can see below.

<?
class AuthenticationHandler {
    public mysqli $conn;
    function authenticate() {
        $user = $_POST['user'];
        $pass = $_POST['pass'];
        $authenticated = false;
        $query = "SELECT * FROM users WHERE user = '" . $user . "' AND pass = '" . $pass . "'";
        $stmt = $conn->query($query); // Noncompliant
        if ($stmt->num_rows == 1) {
          $authenticated = true;
        }
        return $authenticated;
    }
}
?>

What language is this for?
PHP

Which rule?
Database queries should not be vulnerable to injection attacks - S3649

Why do you believe it’s a false-positive/false-negative?
The pieces of code contain SQL Injection vulnerabilities.

Are you using
SonarQube Enterprise Version 9.9.0.65466

Thank You,
Marcos Ferreira

3 Likes

Maybe this issue also affects rule 2077 (Formatting SQL queries is security-sensitive) PHP static code analysis: Formatting SQL queries is security-sensitive

1 Like

Hello Marcos !

Thanks a lot for your messages.

First case: $conn->query($sql)

I cannot answer this case because I do not know which type is $conn. Can you tell me what its type is?

Second case: SQLite3

I can confirm that we do not support SQLite3::querySingle yet. I created an internal ticket to research and implement this. Thanks!

Third case. mysqli

Thanks for pointing out that this code is from our non-compliant samples. I spotted our typo there:

This should be changed

to this:

$stmt = $this->conn->query($query); // Noncompliant

I then get the issue:

I created an internal ticket to handle that.

Thanks a lot for your feedback!

Have a good day,

Loris

Hello @Loris_Sierra

Thank you for your answers.

About the first case: $conn->query($sql)
The connection could be:

$conn = new mysqli($servername, $username, $password, $dbname);

I did the test on my side, and when I added it to the same file, it now showed the vulnerability “Change this code to not construct SQL queries directly from user-controlled data.” which is good.

However, the point here is that if I don’t have the connection on the same file, it does not show this problem in constructing SQL queries using user-controlled data. Is this an expected behavior?

Second case: SQLite3
Is possible to share the ticket to track it?

Third case. mysqli
Did you create a ticket for this one as well?

Thank you,
Marcos

Hey Marcos,

Thanks for clarifying your issue. About the first case, it’s an issue that we know and track since last September. It is about analyzing the use of global variables with include-like functions.

As the three tickets corresponding to your three issues are located in Jira boards that contain sensitive data, they are private. We cannot give you a link to track them, but rest assured we work hard on these. Multiple teams are involved.

Thanks again for your feedback, and have a good weekend.

Loris

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.