Sonarcloud doesn't get new SQL Injection on php (Laravel)

[Ichenn (Tristan)]

Hi community,

I’ve encountered a weird behaviour with the sonarcloud scanner.

SonarCloud did analyse correctly the first time a lot of new sql injection we didn’t see in our code which is awesome!
and after resolving those issues, we wanted to make sure in case a new mistake was made sonarcloud would block the PR on bitbucket.

Unfortunately even by adding few new fake sql injection, sonarcloud wasn’t able to warn us on PR.

Here are the exemple of new code we put.

$join->on('users.id', '=', 'role_user.user_id')
    ->whereRaw("role_user.active = true and role_user.instance_id = " . InstanceHelper::getInstanceId()
    . " AND role_user.role_id = '" . $request->value . "'");

Route::get('test-secu/{evil_variable}', function ($evil_variable) {
    echo print_r(DB::table('users')->whereRaw('first_name = ' . $evil_variable . "'"), true);
});

Thank you a lot

[Karim_El_Ouerghemmi]

Hey @Ichenn,

Welcome to the community!

Nice to read that you were able to identify and fix many SQL injections in your code base.

Regarding your second example, it is strange that you are not seeing a vulnerability raised in it. That case should be supported. I did copy-paste the example, scanned it, and a vulnerability was correctly raised (link).

This makes me suspect that the reason you’re not seeing the vulnerability is that you are trying to find it in a file which is recognized as a test file. For test files, only specific rules run to avoid noise.

Regarding your first example, I cannot verify without having more context of the snippet (i.e., where $join, and $request come from). But, chances are high that the reason you’re not seeing vulnerabilities is the one stated above. Please verify that first.

Best,
Karim.

[Ichenn (Tristan)]

Hey @Karim_El_Ouerghemmi,

Seems that you were really close of the truth
The example that you tested out is actually not cover by our sonarcloud settings. Indeed my bad for testing sonarcloud at the wrong place.

And for other examples it seems that on join it doesn’t really detect it even if I clearly use a user-controlled variable. (Not a test file and cover by scanner settings)

But I did tested out with another one

$object = ObjectClass::with('act_attribute')->whereRaw("id = '" . $request->id . "'")->first();

And sonarcloud could detect it.

Should we investigate more on those join case?, such as:

$object->select('users.*')->join('role', function ($join) {
    $join->on('users.id', '=', 'role.user_id')
    ->whereRaw("role.status = '" . $request->roleStatus . "'");
})

Thanks anyway for your great and quick insight !

[Karim_El_Ouerghemmi]

Hey @Ichenn,

Good that you found the reason for having no issues raised at all

For the case with the advanced join clause, I can say that, for PHP taint analysis, we do have some limitations we are aware of related to anonymous functions. These limitations are for sure one reason that the issue is not and cannot be raised. Unfortunately, I have no estimated timeframe for when this will be fixed.

Thank you very much for reporting this!

Best,
Karim.