dev/69: fix for broken mailing URLS with additional parameters

[jaapjansma]

In my mailing I have the following url: https://my-site.org/form?cid={contact.id}&{contact.checksum}

By default this url is replaced to https://my-site.org/civicrm/mailing/url?u=2&qid=3&cid=..&cs=...

Civi Proxy then translate this to: `https://proxy.my-site.org/url.php?u=2&qid=3&cid=...&cs=...

Expected results

Opening the CiviProxy url would lead to https://my-site.org/form?cid=...&cs=...

Actual results

Opening the CiviProxy url leads to https://my-site.org/form

Which is without the cid and the cs parameter.

Possible cause

CiviProxy does reomve the additional query parameters (such as cid and cs)

See as well #69

[bjendres]

Hi @jaapjansma. Thanks for the submission.

@pbatroff and I talked about this and we feel uncomfortable to allow any parameters in that call (which is of course what the * stands for) just because it is convenient. This would allow malicious parameters to be passed on to CiviCRM.

My suggestion would be to restrict to the known parameters (e.g. ..., 'cid' => '*', 'cs' => '*',...), could you adjust the PR if possible?

Ideally this would be configurable via the configuration file like here, but I can understand if you don't want to do this right now. This would, however, allow your '*' => 'string' to be applied only in your local CiviProxy installation.

[jaapjansma]

@bjendres I do think you don't understand the issue here. It is could be any parameter. It depends on the url the user has entered in CiviMail. E.g. if a user would enter a url like https://google.com?q=abc then CiviCRM translates this https://my-site.org/civicrm/mailing/url?u=..&qid=...&q=abc

The url stored in CiviCRM database corresponding to u and qid is this: https://google.com without the q=abc

@bjendres do you have any other suggestion how we can solve this?

[pbatroff]

Hi @jaapjansma ,

we realize it could be any parameter in there, but in the spirit of the security concept of CiviProxy we would propose a whitelisting approach, that can incorporate the wildcard, but doesn't have it hard coded.

So the array $valid_parameters in url.php should be refactored to the config.php, and documentation should be added in the config.php.dist file to let the users know of the configuration option.

Then it's up to the user to whitelist everything (e.g. '*'), or only whitelist certain parameters. I can start implementing this next week

[jaapjansma]

@pbatroff yes that make sense to me