Merge branch 'SvenRtbg-fix-issue-15'

Closes #16

Author: akrabat

README.md

@@ -15,7 +15,7 @@ Note that the proxy headers are only checked if the first parameter to the const
 
 **Trusted Proxies**
 
-You can set a list of proxies that are trusted as the second constructor parameter. If this list is set, then the proxy headers will only be checked if the `REMOTE_ADDR` is in the trusted list.
+If you configure to check the proxy headers (first parameter is `true`), you have to provide an array of trusted proxies as the second parameter. If the array is empty, the proxy headers will always be evaluated. If the array is not empty, it must contain strings with IP addresses, one of them must be the `$_SERVER['REMOTE_ADDR']` variable in order to allow evaluating the proxy headers - otherwise the `REMOTE_ADDR` itself is returned.
 
 **Attribute name**
 
@@ -25,6 +25,12 @@ By default, the name of the attribute is '`ip_address`'. This can be changed by
 
 By default, this middleware checks the 'Forwarded', 'X-Forwarded-For', 'X-Forwarded', 'X-Cluster-Client-Ip' and 'Client-Ip' headers. You can replace this list with your own using the fourth constructor parameter.
 
+## Security considerations
+
+A malicious client may send any header to your proxy, including any proxy headers, containing any IP address. If your proxy simply adds another IP address to the header, an attacker can send a fake IP. Make sure to setup your proxy in a way that removes any sent (and possibly faked) headers from the original request and replaces them with correct values (i.e. the currently used `REMOTE_ADDR` on the proxy server).
+
+This library cannot by design ensure you get correct and trustworthy results if your network environment isn't setup properly.
+
 ## Installation
 
 `composer require akrabat/ip-address-middleware`

src/IpAddress.php

@@ -58,10 +58,14 @@ class IpAddress implements MiddlewareInterface
      */
     public function __construct(
         $checkProxyHeaders = false,
-        array $trustedProxies = [],
+        array $trustedProxies = null,
         $attributeName = null,
         array $headersToInspect = []
     ) {
+        if ($checkProxyHeaders && $trustedProxies === null) {
+            throw new \InvalidArgumentException('Use of the forward headers requires an array for trusted proxies.');
+        }
+
         $this->checkProxyHeaders = $checkProxyHeaders;
         $this->trustedProxies = $trustedProxies;
 
@@ -127,10 +131,14 @@ protected function determineClientIpAddress($request)
             }
         }
 
-        if ($this->checkProxyHeaders
-            && !empty($this->trustedProxies)
-            && in_array($ipAddress, $this->trustedProxies)
-        ) {
+        $checkProxyHeaders = $this->checkProxyHeaders;
+        if ($checkProxyHeaders && !empty($this->trustedProxies)) {
+            if (!in_array($ipAddress, $this->trustedProxies)) {
+                $checkProxyHeaders = false;
+            }
+        }
+
+        if ($checkProxyHeaders) {
             foreach ($this->headersToInspect as $header) {
                 if ($request->hasHeader($header)) {
                     $ip = $this->getFirstIpAddressFromHeader($request, $header);

tests/IpAddressTest.php

@@ -71,7 +71,7 @@ public function testIpIsNullIfMissing()
 
     public function testXForwardedForIp()
     {
-        $middleware = new IPAddress(true, ['192.168.1.1']);
+        $middleware = new IPAddress(true, []);
 
         $request = ServerRequestFactory::fromGlobals([
             'REMOTE_ADDR' => '192.168.1.1',
@@ -131,7 +131,7 @@ public function testProxyIpIsIgnored()
 
     public function testHttpClientIp()
     {
-        $middleware = new IPAddress(true, ['192.168.1.1']);
+        $middleware = new IPAddress(true, []);
 
         $request = ServerRequestFactory::fromGlobals([
             'REMOTE_ADDR' => '192.168.1.1',
@@ -151,7 +151,7 @@ public function testHttpClientIp()
 
     public function testXForwardedForIpV6()
     {
-        $middleware = new IPAddress(true, ['192.168.1.1']);
+        $middleware = new IPAddress(true, []);
 
         $request = ServerRequestFactory::fromGlobals([
             'REMOTE_ADDR' => '192.168.1.1',
@@ -171,7 +171,7 @@ public function testXForwardedForIpV6()
 
     public function testXForwardedForWithInvalidIp()
     {
-        $middleware = new IPAddress(true, ['192.168.1.1']);
+        $middleware = new IPAddress(true, []);
 
         $request = ServerRequestFactory::fromGlobals([
             'REMOTE_ADDR' => '192.168.1.1',
@@ -231,7 +231,7 @@ public function testXForwardedForIpWithUntrustedProxy()
 
     public function testForwardedWithMultipleFor()
     {
-        $middleware = new IPAddress(true, ['192.168.1.1']);
+        $middleware = new IPAddress(true, []);
 
         $request = ServerRequestFactory::fromGlobals([
             'REMOTE_ADDR' => '192.168.1.1',
@@ -251,7 +251,7 @@ public function testForwardedWithMultipleFor()
 
     public function testForwardedWithAllOptions()
     {
-        $middleware = new IPAddress(true, ['192.168.1.1']);
+        $middleware = new IPAddress(true, []);
 
         $request = ServerRequestFactory::fromGlobals([
             'REMOTE_ADDR' => '192.168.1.1',
@@ -271,7 +271,7 @@ public function testForwardedWithAllOptions()
 
     public function testForwardedWithWithIpV6()
     {
-        $middleware = new IPAddress(true, ['192.168.1.1']);
+        $middleware = new IPAddress(true, []);
 
         $request = ServerRequestFactory::fromGlobals([
             'REMOTE_ADDR' => '192.168.1.1',
@@ -294,7 +294,7 @@ public function testCustomHeader()
         $headersToInspect = [
             'Foo-Bar'
         ];
-        $middleware = new IPAddress(true, ['192.168.0.1'], null, $headersToInspect);
+        $middleware = new IPAddress(true, [], null, $headersToInspect);
 
         $request = ServerRequestFactory::fromGlobals([
             'REMOTE_ADDR' => '192.168.0.1',
@@ -333,4 +333,10 @@ public function handle(ServerRequestInterface $request): ResponseInterface
 
         $this->assertSame("Hello World", (string) $response->getBody());
     }
+
+    public function testNotGivingAProxyListShouldThrowException()
+    {
+        $this->expectException(\InvalidArgumentException::class);
+        new IpAddress(true);
+    }
 }