diff --git a/lib/private/AppFramework/Http/Request.php b/lib/private/AppFramework/Http/Request.php index ea4cbf3b2fba1..4cd85f16437f8 100644 --- a/lib/private/AppFramework/Http/Request.php +++ b/lib/private/AppFramework/Http/Request.php @@ -637,9 +637,11 @@ public function getRemoteAddress(): string { // only have one default, so we cannot ship an insecure product out of the box ]); - foreach ($forwardedForHeaders as $header) { + // Read the x-forwarded-for headers and values in reverse order as per + // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-For#selecting_an_ip_address + foreach (array_reverse($forwardedForHeaders) as $header) { if (isset($this->server[$header])) { - foreach (explode(',', $this->server[$header]) as $IP) { + foreach (array_reverse(explode(',', $this->server[$header])) as $IP) { $IP = trim($IP); // remove brackets from IPv6 addresses @@ -647,6 +649,10 @@ public function getRemoteAddress(): string { $IP = substr($IP, 1, -1); } + if ($this->isTrustedProxy($trustedProxies, $IP)) { + continue; + } + if (filter_var($IP, FILTER_VALIDATE_IP) !== false) { return $IP; } diff --git a/tests/lib/AppFramework/Http/RequestTest.php b/tests/lib/AppFramework/Http/RequestTest.php index a4a03b6479cdc..78f7fc11bc51a 100644 --- a/tests/lib/AppFramework/Http/RequestTest.php +++ b/tests/lib/AppFramework/Http/RequestTest.php @@ -481,7 +481,33 @@ public function testGetRemoteAddressWithSingleTrustedRemote() { $this->stream ); - $this->assertSame('10.4.0.5', $request->getRemoteAddress()); + $this->assertSame('10.4.0.4', $request->getRemoteAddress()); + } + + public function testGetRemoteAddressWithMultipleTrustedRemotes() { + $this->config + ->expects($this->exactly(2)) + ->method('getSystemValue') + ->willReturnMap([ + ['trusted_proxies', [], ['10.0.0.2', '::1']], + ['forwarded_for_headers', ['HTTP_X_FORWARDED_FOR'], ['HTTP_X_FORWARDED']], + ]); + + $request = new Request( + [ + 'server' => [ + 'REMOTE_ADDR' => '10.0.0.2', + 'HTTP_X_FORWARDED' => '10.4.0.5, 10.4.0.4, ::1', + 'HTTP_X_FORWARDED_FOR' => '192.168.0.233' + ], + ], + $this->secureRandom, + $this->config, + $this->csrfTokenManager, + $this->stream + ); + + $this->assertSame('10.4.0.4', $request->getRemoteAddress()); } public function testGetRemoteAddressIPv6WithSingleTrustedRemote() { @@ -510,7 +536,7 @@ public function testGetRemoteAddressIPv6WithSingleTrustedRemote() { $this->stream ); - $this->assertSame('10.4.0.5', $request->getRemoteAddress()); + $this->assertSame('10.4.0.4', $request->getRemoteAddress()); } public function testGetRemoteAddressVerifyPriorityHeader() { @@ -524,9 +550,9 @@ public function testGetRemoteAddressVerifyPriorityHeader() { ->method('getSystemValue') ->with('forwarded_for_headers') ->willReturn([ - 'HTTP_CLIENT_IP', + 'HTTP_X_FORWARDED', 'HTTP_X_FORWARDED_FOR', - 'HTTP_X_FORWARDED' + 'HTTP_CLIENT_IP', ]); $request = new Request( @@ -557,9 +583,9 @@ public function testGetRemoteAddressIPv6VerifyPriorityHeader() { ->method('getSystemValue') ->with('forwarded_for_headers') ->willReturn([ - 'HTTP_CLIENT_IP', + 'HTTP_X_FORWARDED', 'HTTP_X_FORWARDED_FOR', - 'HTTP_X_FORWARDED' + 'HTTP_CLIENT_IP' ]); $request = new Request(