From 724e013663d1e6585939818d0efa1e966aa02383 Mon Sep 17 00:00:00 2001 From: seagull Date: Fri, 12 Feb 2021 13:18:53 +0300 Subject: [PATCH] Fixed issue where wildcard list was ignored except for the first element. Add tests --- src/IpAddress.php | 44 +++++++++++++++++++++----------------- tests/IpAddressTest.php | 47 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 19 deletions(-) diff --git a/src/IpAddress.php b/src/IpAddress.php index 409366d..aaeeabd 100644 --- a/src/IpAddress.php +++ b/src/IpAddress.php @@ -172,16 +172,15 @@ protected function determineClientIpAddress($request) } } - $checkProxyHeaders = $this->checkProxyHeaders; - if ($checkProxyHeaders) { + $checkProxyHeaders = false; + if ($this->checkProxyHeaders) { // Exact Match - if ($this->trustedProxies && !in_array($ipAddress, $this->trustedProxies)) { - $checkProxyHeaders = false; + if ($this->trustedProxies && in_array($ipAddress, $this->trustedProxies)) { + $checkProxyHeaders = true; } // Wildcard Match - if ($checkProxyHeaders && $this->trustedWildcard) { - $checkProxyHeaders = false; + if ($this->checkProxyHeaders && $this->trustedWildcard) { // IPv4 has 4 parts separated by '.' // IPv6 has 8 parts separated by ':' if (strpos($ipAddress, '.') > 0) { @@ -196,19 +195,22 @@ protected function determineClientIpAddress($request) if (count($proxy) !== $parts) { continue; // IP version does not match } + $match = true; foreach ($proxy as $i => $part) { if ($part !== '*' && $part !== $ipAddrParts[$i]) { - break 2;// IP does not match, move to next proxy + $match = false; + break;// IP does not match, move to next proxy } } - $checkProxyHeaders = true; - break; + if ($match) { + $checkProxyHeaders = true; + break; + } } } // CIDR Match - if ($checkProxyHeaders && $this->trustedCidr) { - $checkProxyHeaders = false; + if ($this->checkProxyHeaders && $this->trustedCidr) { // Only IPv4 is supported for CIDR matching $ipAsLong = ip2long($ipAddress); if ($ipAsLong) { @@ -220,15 +222,19 @@ protected function determineClientIpAddress($request) } } } - } - if ($checkProxyHeaders) { - foreach ($this->headersToInspect as $header) { - if ($request->hasHeader($header)) { - $ip = $this->getFirstIpAddressFromHeader($request, $header); - if ($this->isValidIpAddress($ip)) { - $ipAddress = $ip; - break; + if (!$this->trustedProxies && !$this->trustedWildcard && !$this->trustedCidr) { + $checkProxyHeaders = true; + } + + if ($checkProxyHeaders) { + foreach ($this->headersToInspect as $header) { + if ($request->hasHeader($header)) { + $ip = $this->getFirstIpAddressFromHeader($request, $header); + if ($this->isValidIpAddress($ip)) { + $ipAddress = $ip; + break; + } } } } diff --git a/tests/IpAddressTest.php b/tests/IpAddressTest.php index 9a185a4..ab35ad6 100644 --- a/tests/IpAddressTest.php +++ b/tests/IpAddressTest.php @@ -316,6 +316,53 @@ public function handle(ServerRequestInterface $request): ResponseInterface $this->assertSame("Hello World", (string) $response->getBody()); } + public function testIpCidrListMatch() + { + $matches = [ + '192.16.238.184/24', // negative match + '10.11.0.0/16', // positive match + ]; + $middleware = new IPAddress(true, $matches); + $env = [ + 'REMOTE_ADDR' => '10.11.156.95', + 'HTTP_X_FORWARDED_FOR' => '123.4.5.6', + ]; + $ipAddress = $this->simpleRequest($middleware, $env); + $this->assertSame('123.4.5.6', $ipAddress, "Testing CIDR: " . implode(', ', $matches)); + } + + public function testIp4WildcardsMatch() + { + $matches = [ + '192.168.*.*', // negative match + '10.0.238.*', // negative match + '10.11.*.*', // positive match + ]; + $middleware = new IPAddress(true, $matches); + $env = [ + 'REMOTE_ADDR' => '10.11.156.95', + 'HTTP_X_FORWARDED_FOR' => '123.4.5.6', + ]; + $ipAddress = $this->simpleRequest($middleware, $env); + $this->assertSame('123.4.5.6', $ipAddress, "Testing wildcard: " . implode(', ', $matches)); + } + + public function testIp4MultipleTypesMatch() + { + $matches = [ + '192.168.0.1', // negative match + '10.0.238.*', // negative match + '10.11.0.0/16', // positive match + ]; + $middleware = new IPAddress(true, $matches); + $env = [ + 'REMOTE_ADDR' => '10.11.156.95', + 'HTTP_X_FORWARDED_FOR' => '123.4.5.6', + ]; + $ipAddress = $this->simpleRequest($middleware, $env); + $this->assertSame('123.4.5.6', $ipAddress, "Testing proxies: " . implode(', ', $matches)); + } + public function testNotGivingAProxyListShouldThrowException() { $this->expectException(\InvalidArgumentException::class);