trusted proxies validation is invalid and allows nonsense values
How to use GitHub
- Please use the 👍 reaction to show that you are affected by the same issue.
- Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
- Subscribe to receive notifications on status change and new comments.
Steps to reproduce
- add trusted_proxies with IPv6 syntax
- create invalid notify_push config e.g. wrong trusted_proxies or missing headers
- perform either
occ notify_push:setup ...orocc notify_push:self-test - a warning about as invalid IPv6 proxy is shown
Expected behaviour
according to docs https://docs.nextcloud.com/server/stable/admin_manual/configuration_server/reverse_proxy_configuration.html reverse proxy could be IPv4 addresses and ranges in CIDR notation, IPv6 addresses and ranges in CIDR notation. the check should reflect this fact.
- IPv6 syntax should be valid
- IPv4 CIDR syntax should be verified properly (current regex allows nonsense values like 333.444.555.666/77)
Actual behaviour
🗴 push server is not a trusted proxy by Nextcloud or another proxy in the chain.
Nextcloud resolved the following client address for the test request: "Invalid response when testing if the push server is a trusted proxy: invalid IP address syntax" instead of the expected "1.2.3.4" test value.
The following trusted proxies are currently configured: "172.16.0.0/12", "192.168.0.0/16", "10.0.0.0/8", "fc00::/7", "fe80::/10", "2001:db8::/32"
of which the following seem to be invalid: "fc00::/7", "fe80::/10", "2001:db8::/32"
The following x-forwarded-for header was received by Nextcloud: ""
from the following remote:
the problem results from $cidrre = '/^([0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3})\/([0-9]{1,2})$/'; regex used in SelfTest.php which doesn't match IPv6 (and is not strict enough to match only valid IPv4 syntax)
https://github.com/nextcloud/notify_push/blob/f71fcbc1a0b348d7b4417bb65cb624bd79eb22bc/lib/SelfTest.php#L240C1-L248C3
private function isValidProxyConfig(string $proxyConfig): bool {
$cidrre = '/^([0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3})\/([0-9]{1,2})$/';
if (filter_var($proxyConfig, FILTER_VALIDATE_IP) !== false) {
return true;
} else {
return (bool)preg_match($cidrre, $proxyConfig);
}
}
there are better regex like https://uibakery.io/regex-library/ip-address-regex-php or better check functions like https://gist.github.com/pavinjosdev/cb1d636ea9dc2bd201d54107d10650c5
Note that isValidProxyConfig is only used for generating the diagnostics when a trusted proxy issue is detected. It has no impact on whether or not the trusted setup check succeeds or fails.
(it should still be fixed ofc)