Prevent setting pk_ref cookie with an empty string; it is perceived as sql injection attempt

[atom-box]

Expected: pk_ref will never be set with "" property.

Actual: pk_ref is sometimes set with: ["", "", 1234567890, "https://www.example.com/"].

Why this is a problem: Security software throws a false positive, seeing this as a SQL injection attempt.

Here is the original email from our user:

We integrated Matomo Analytics in a react-application that is hosted in azure. When redirected to this page (https://validation.EXAMPLE.ai) from the main homepage of our company (https://www.EXAMPLE.dk) via a link a cookie called _pk_ref.67.352a is being created by Matomo Analytics like so: ["", "", 1234567890, "https://www.EXAMPLE.dk/"].

This causes our application gateway to return HTTP 403 since the web application firewall running in the background assumes this cookie contains an sql injection attempt. This is due to the two double quotes in the cookie. Since the azure web application firewall which we use is commonly used and we do not want to disable checking for sql injections entirely could you provide us with an explanation as to how and why the cookie is created this way and provide potential a fix. Thank you in advance.

--

[sgiehl]

@atom-box We are setting a json encoded array as cookie value. If no campaign details were provided for the visit, the first two values are empty strings. Which ends up in your example. We can't easily change that and if we would decide to do so, this can't be done before a new major release. Anyway, I'm not sure if their WAF might be a bit too restrictive in that case. Maybe they should disable that rule for Matomo.

[michalkleiner]

Could we, in that case, not set the cookie at all and treat a cookie that is not set the same way as one without the values? Or send null string instead indicating an empty value, @sgiehl? That sounds like a minor version enhancement or possibly even a patch fix if the logic processing the cookies treated the empty string and the null value the same way.

[sgiehl]

As that's part of the tracking, the code would need to be able to handle the old and the new style of cookies. Setting an empty cookie wouldn't be the same, as the third and fourth value of the json encoded array are set and required. We could set null instead and implement a handling for that. But t.b.h. I'm pretty sure, that might only fix one of the problems a tracking behind a very restrictive waf would have.

[julipp]

Hi, I originally raised this issue through the support page The ruleset for the azure waf is OWASP 3.1 CRS which comes with a range of predefined rules to detect security anomalies. It is common standard choice for a ruleset for an azure waf. We could exclude requests containing the _pk_ref cookie from certain rule checks - I will discuss this internally with the responsible person and I hope that this should resolve the issue for us. Whether the OWASP 3.1 CRS rules are too restrictive for you to want to support tracking behind them out of the box is open for discussion and ultimately your choice.

[sgiehl]

@tsteur Do you think this should be fixed?

[tsteur]

@sgiehl I guess it depends how easy it is to fix. It would be nice if it just worked out of the box with such WAF rules. If it's too much effort, then I would maybe rather wait until more people run into this issue. The least we should do though, is create an FAQ around Matomo and WAF rules. So it's clear that certain rules may be causing issues and trigger eg 403 response codes etc.

[sgiehl]

Guess passing null instead of an empty string should be implemented quite easily.

[tsteur]

Guess passing null instead of an empty string should be implemented quite easily.

We might want to check if this causes similar issues

[Jhfelectric]

Hello, I have the same issue on my company's WAF. I don't have access myself but here is what was reported to me from the log (anonymized):

2023:10:18-12:36:08 securitysrv1-2 httpd[18459]: [security2:error] [pid 18459:tid 3861777216] [client 84.123.456.789:36647] [client 84.123.456.789] ModSecurity: Warning. Pattern match "([\\~\\!\\@\\#\\$\\%\\^\\&\\\\(\\)\\-\\+\\=\\{\\}\\[\\]\\|\\:\\;\"\\'\\\xc2\xb4\\\xe2\x80\x99\\\xe2\x80\x98\\`\\<\\>].?){8,}" at REQUEST_COOKIES:_pk_ref.6.8e6a. [file "/usr/apache/conf/waf/modsecurity_crs_sql_injection_attacks.conf"] [line "157"] [id "981172"] [rev "2"] [msg "Restricted SQL Character Anomaly Detection Alert - Total # of special characters exceeded"] [data "Matched Data: - found within REQUEST_COOKIES:_pk_ref.6.8e6a: [\x22\x22,\x22\x22,1697624257,\x22https://some.domain.tld/\x22]"] [ver "OWASP_CRS/2.2.7"] [maturity "9"] [accuracy "8"] [tag "OWASP_CRS/WEB_ATTACK/SQL_INJECTION"] [hostname "other.domain.tld"] [uri "/"] [unique_id "ZS-1GId9oroUJOVy24FkHwAAAOc"], referer: https://other.domain.tld/

My ITs have created some sort of exception for the empty string cookie. @tsteur so more people see this occurring, and yes, a FAQ would have saved me 2 days ;)

Thanks for the hard work ! Julien

[gheisz]

We seem to have the same problem.

[Gazymodo]

Exactly same issue for us!

[Gazymodo]

Guess passing null instead of an empty string should be implemented quite easily.

Any idea if this will be fixed. We are trying to solve this but may have to remove matomo for some of our services at the moment.

[sgiehl]

Hey @Gazymodo, we currently don't have enough capacity to work on this one. I'll nevertheless create a draft PR with the changes, that might be required to fix this. We though might not be able to test properly if that would solve the problem. If you have time to validate and test the changes, that might help a lot to get it merged.

[Gazymodo]

Hey @Gazymodo, we currently don't have enough capacity to work on this one. I'll nevertheless create a draft PR with the changes, that might be required to fix this. We though might not be able to test properly if that would solve the problem. If you have time to validate and test the changes, that might help a lot to get it merged.

Thank you. I am not the matomo guy at our place but I asked to get it implemented to be able to test. Will keep you posted. If anyone else could do the same, it would be great