SonarQube PHP - How to modify URL validation and tainting detection logic

[Takis (P Andreou)]

We have a case that we would like to configure how the tainting detection works for URLs (PHP) as an allowed list of URLs is not something we can easily define and maintain as it varies from our clients. The script accepts a URL parameter that is associated with image source URLs.

In details.

Here is the flagging PHP code, that’s used as intended to capture the passed-in URL of image sources.

$url = $_GET['url'];

The $url is propagated down to other functions. SonarQube flags it as a ‘Server-side requests should not be vulnerable to forging attacks’ attack and advises us to create an allowed list.

$allowedList = array(
  "https://www.sonarsource.com"
);
$url = $_GET["url"];
if (in_array($url, $allowedList)) {
 ...

The URL varies (as we have many clients with different domains for their images) so I tried to use the built-in PHP function filter_var to sanitise the $_GET["url"] input before assigning it to the $url variable.

e.g.,

// Image URL validation
if (filter_var($_GET['url'], FILTER_VALIDATE_URL) 
    || filter_var(urldecode($_GET['url']), FILTER_VALIDATE_URL)) {
    call_custom_error_func('Image URL is not valid');
}

That didn’t work. SonarQube still flags it and expects an allowed list. Is anything we can do to provide a custom rule to “tell” SonarQube that URL validation is more than enough here and an allowed list is probably an overkill for us. Any docs references around this would be great!

Thanks in advance.

[Colin]

Hey there.

My (limited) understanding is that we consider filter_var a common sanitizer – so I’m not sure what’s going on here. Can you tell us what SonarQube version you’re using?

[Hendrik_Buchwald]

Hello,

The main reason why the issue still shows up here is that we have defined filter_var as a sanitizer but not as a validator. That means, we only consider the return value of filter_var safe. This is something that can be improved and I will create a ticket for it, as it depends on the filter if it is a validator or sanitizer.

It does not seem like a valid protection against SSRF though. Maybe you can tell me a bit more about what you are trying to do with the validation code?

For the case that you really want to allow requests to arbitrary URLs (which is a valid use-case), I would recommend you to send all requests through a proxy that is separated from all internal services. We will still raise an issue but you can just flag it as a “won’t fix”.

[Takis (P Andreou)]

Thanks both for the responses.

Yes, it will be great to improve the behaviour of the filter_val in SonarQube and add also documentation for it. Thanks for looking into it.

What I am hoping to achieve here is to find a way to tell SonarQube that using the filter_val function is enough to both sanitise and validate (specific in this use case) the $_GET['url'] as a URL string before it is propagated further in the script(s). This is part of a micro service. I understand that an allowed list is very important for other cases but this service accepts arbitrary URLs.

Perhaps, if there is not an easy way to flag these cases (per project); e.g., with a custom variation of this rule that “overwrites” the default functionality; we can just mark it as “won’t fix” for now.

[Hendrik_Buchwald]

You are welcome. If you have SonarQube Enterprise Edition or higher you are able to specify your own configuration for the taint-analyzer. In that case you could specify filter_var as a validator. With other SonarQube versions that is not possible and you will have to use “won’t fix” reviews until the change is implemented on our side. I can not give any estimate when this will be the case though. I have created the ticket but it can take a few months until it gets implemented.

Alternatively, you can also consider disabling the SSRF rule for this project if you don’t consider it a valid problem in the environment.

[Takis (P Andreou)]

Thanks Hendrik, I am passing you valuable feedback to the rest of the team.