Skip to content

Commit 156b034

Browse files
committed
fix(settings): use CIDR-aware proxy detection in ForwardedForHeaders setup check
The setup check used in_array() with strict comparison to determine if REMOTE_ADDR matched a trusted proxy entry. This cannot match CIDR ranges (e.g., 172.16.0.0/12) because the raw IP and the CIDR string are never equal. Replace in_array() with a check that compares the raw REMOTE_ADDR against the resolved address from getRemoteAddress(), which already handles CIDR matching internally via IpUtils::checkIp(). Also add a test case for large CIDR (/12) matching to prevent future regressions. Fixes: #60287 Signed-off-by: Arya Rizky <arya@algojogacor.dev>
1 parent 88b79c6 commit 156b034

2 files changed

Lines changed: 29 additions & 20 deletions

File tree

apps/settings/lib/SetupChecks/ForwardedForHeaders.php

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -72,16 +72,16 @@ public function run(): SetupResult {
7272
);
7373
}
7474

75-
if (\in_array($remoteAddress, $trustedProxies, true) && ($remoteAddress !== '127.0.0.1')) {
76-
if ($remoteAddress !== $detectedRemoteAddress) {
77-
/* Remote address was successfuly fixed */
78-
return SetupResult::success($this->l10n->t('Your IP address was resolved as %s', [$detectedRemoteAddress]));
79-
} else {
80-
return SetupResult::warning(
81-
$this->l10n->t('The reverse proxy header configuration is incorrect, or you are accessing Nextcloud from a trusted proxy. If not, this is a security issue and can allow an attacker to spoof their IP address as visible to the Nextcloud.'),
82-
$this->urlGenerator->linkToDocs('admin-reverse-proxy')
83-
);
84-
}
75+
if ($remoteAddress !== $detectedRemoteAddress && $remoteAddress !== '127.0.0.1') {
76+
/* Remote address was successfully resolved via trusted proxy */
77+
return SetupResult::success($this->l10n->t('Your IP address was resolved as %s', [$detectedRemoteAddress]));
78+
}
79+
80+
if (!empty($trustedProxies) && $remoteAddress === $detectedRemoteAddress && $remoteAddress !== '' && $remoteAddress !== '127.0.0.1') {
81+
return SetupResult::warning(
82+
$this->l10n->t('The reverse proxy header configuration is incorrect, or you are accessing Nextcloud from a trusted proxy. If not, this is a security issue and can allow an attacker to spoof their IP address as visible to the Nextcloud.'),
83+
$this->urlGenerator->linkToDocs('admin-reverse-proxy')
84+
);
8585
}
8686

8787
/* Either not enabled or working correctly */

tests/lib/AppFramework/Http/RequestTest.php

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -639,16 +639,25 @@ public static function dataGetRemoteAddress(): array {
639639
],
640640
'192.168.0.233',
641641
],
642-
'IPv4 matching CIDR of trusted proxy' => [
643-
[
644-
'REMOTE_ADDR' => '192.168.3.99',
645-
'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4',
646-
'HTTP_X_FORWARDED_FOR' => '192.168.0.233',
647-
],
648-
['192.168.2.0/24'],
649-
['HTTP_X_FORWARDED_FOR'],
650-
'192.168.3.99',
651-
],
642+
\t\t\t'IPv4 matching CIDR of trusted proxy' => [
643+
\t\t\t\t[
644+
\t\t\t\t\t'REMOTE_ADDR' => '192.168.3.99',
645+
\t\t\t\t\t'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4',
646+
\t\t\t\t\t'HTTP_X_FORWARDED_FOR' => '192.168.0.233',
647+
\t\t\t\t],
648+
\t\t\t\t['192.168.2.0/24'],
649+
\t\t\t\t['HTTP_X_FORWARDED_FOR'],
650+
\t\t\t\t'192.168.3.99',
651+
\t\t\t],
652+
\t\t\t'IPv4 matching large CIDR (/12) of trusted proxy' => [
653+
\t\t\t\t[
654+
\t\t\t\t\t'REMOTE_ADDR' => '172.21.0.7',
655+
\t\t\t\t\t'HTTP_X_FORWARDED_FOR' => '10.0.0.42',
656+
\t\t\t\t],
657+
\t\t\t\t['172.16.0.0/12'],
658+
\t\t\t\t['HTTP_X_FORWARDED_FOR'],
659+
\t\t\t\t'10.0.0.42',
660+
\t\t\t],
652661
'IPv6 matching CIDR of trusted proxy' => [
653662
[
654663
'REMOTE_ADDR' => '2001:db8:85a3:8d3:1319:8a21:370:7348',

0 commit comments

Comments
 (0)