nextcloud / nextcloud/notify_push
trusted proxies validation is invalid and allows nonsense values
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 344
- Forks
- 64
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 5
Description
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)
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
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
The validation is in lib/SelfTest.php, in isValidProxyConfig; start there and reproduce the issue with occ notify_push:setup or occ notify_push:self-test using the trusted proxy examples. Done means valid IPv6 and IPv4 CIDR values are accepted while malformed IPv4 addresses and prefixes are rejected.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100