Merge branch 'security/zf2018-01'

Fixes ZF2018-01
zendframework · Aug 1, 2018 · 6641f4c · 6641f4c
2 parents b3d847a + b28589c
commit 6641f4c
Show file tree

Hide file tree

Showing 5 changed files with 205 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,15 +2,44 @@
 
 All notable changes to this project will be documented in this file, in reverse chronological order by release.
 
-## 2.10.3 - TBD
+## 2.10.3 - 2018-08-01
 
 ### Added
 
 - Nothing.
 
 ### Changed
 
-- Nothing.
+- This release modifies how `Zend\Feed\Pubsubhubbub\AbstractCallback::_detectCallbackUrl()`
+  marshals the request URI. In prior releases, we would attempt to inspect the
+  `X-Rewrite-Url` and `X-Original-Url` headers, using their values, if present.
+  These headers are issued by the ISAPI_Rewrite module for IIS (developed by
+  HeliconTech). However, we have no way of guaranteeing that the module is what
+  issued the headers, making it an unreliable source for discovering the URI. As
+  such, we have removed this feature in this release.
+
+  The method is not called internally. If you are calling the method from your
+  own extension and need support for ISAPI_Rewrite, you will need to override
+  the method as follows:
+
+  ```php
+  protected function _detectCallbackUrl()
+  {
+      $callbackUrl = null;
+      if (isset($_SERVER['HTTP_X_REWRITE_URL'])) {
+          $callbackUrl = $_SERVER['HTTP_X_REWRITE_URL'];
+      }
+      if (isset($_SERVER['HTTP_X_ORIGINAL_URL'])) {
+          $callbackUrl = $_SERVER['HTTP_X_ORIGINAL_URL'];
+      }
+
+      return $callbackUrl ?: parent::__detectCallbackUrl();
+  }
+  ```
+
+  If you use an approach such as the above, make sure you also instruct your web
+  server to strip any incoming headers of the same name so that you can
+  guarantee they are issued by the ISAPI_Rewrite module.
 
 ### Deprecated
 

diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -2,6 +2,7 @@
 <phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          xsi:noNamespaceSchemaLocation="vendor/phpunit/phpunit/phpunit.xsd"
          bootstrap="vendor/autoload.php"
+         backupGlobals="true"
          colors="true">
     <testsuites>
         <testsuite name="zend-feed Test Suite">

diff --git a/src/PubSubHubbub/AbstractCallback.php b/src/PubSubHubbub/AbstractCallback.php
@@ -211,28 +211,31 @@ public function getSubscriberCount()
     protected function _detectCallbackUrl()
     {
         // @codingStandardsIgnoreEnd
-        $callbackUrl = '';
-        if (isset($_SERVER['HTTP_X_ORIGINAL_URL'])) {
-            $callbackUrl = $_SERVER['HTTP_X_ORIGINAL_URL'];
-        } elseif (isset($_SERVER['HTTP_X_REWRITE_URL'])) {
-            $callbackUrl = $_SERVER['HTTP_X_REWRITE_URL'];
-        } elseif (isset($_SERVER['REQUEST_URI'])) {
-            $callbackUrl = $_SERVER['REQUEST_URI'];
-            $scheme = 'http';
-            if ($_SERVER['HTTPS'] == 'on') {
-                $scheme = 'https';
-            }
-            $schemeAndHttpHost = $scheme . '://' . $this->_getHttpHost();
-            if (strpos($callbackUrl, $schemeAndHttpHost) === 0) {
-                $callbackUrl = substr($callbackUrl, strlen($schemeAndHttpHost));
-            }
-        } elseif (isset($_SERVER['ORIG_PATH_INFO'])) {
-            $callbackUrl = $_SERVER['ORIG_PATH_INFO'];
-            if (! empty($_SERVER['QUERY_STRING'])) {
-                $callbackUrl .= '?' . $_SERVER['QUERY_STRING'];
-            }
+        $callbackUrl = null;
+
+        // IIS7 with URL Rewrite: make sure we get the unencoded url
+        // (double slash problem).
+        $iisUrlRewritten = isset($_SERVER['IIS_WasUrlRewritten']) ? $_SERVER['IIS_WasUrlRewritten'] : null;
+        $unencodedUrl    = isset($_SERVER['UNENCODED_URL']) ? $_SERVER['UNENCODED_URL'] : null;
+        if ('1' == $iisUrlRewritten && ! empty($unencodedUrl)) {
+            return $unencodedUrl;
         }
-        return $callbackUrl;
+
+        // HTTP proxy requests setup request URI with scheme and host [and port]
+        // + the URL path, only use URL path.
+        if (isset($_SERVER['REQUEST_URI'])) {
+            $callbackUrl = $this->buildCallbackUrlFromRequestUri();
+        }
+
+        if (null !== $callbackUrl) {
+            return $callbackUrl;
+        }
+
+        if (isset($_SERVER['ORIG_PATH_INFO'])) {
+            return $this->buildCallbackUrlFromOrigPathInfo();
+        }
+
+        return '';
     }
 
     /**
@@ -247,19 +250,19 @@ protected function _getHttpHost()
         if (! empty($_SERVER['HTTP_HOST'])) {
             return $_SERVER['HTTP_HOST'];
         }
-        $scheme = 'http';
-        if ($_SERVER['HTTPS'] == 'on') {
-            $scheme = 'https';
-        }
-        $name = $_SERVER['SERVER_NAME'];
-        $port = $_SERVER['SERVER_PORT'];
-        if (($scheme == 'http' && $port == 80)
-            || ($scheme == 'https' && $port == 443)
+
+        $https  = isset($_SERVER['HTTPS']) ? $_SERVER['HTTPS'] : null;
+        $scheme = $https === 'on' ? 'https' : 'http';
+        $name   = isset($_SERVER['SERVER_NAME']) ? $_SERVER['SERVER_NAME'] : '';
+        $port   = isset($_SERVER['SERVER_PORT']) ? (int) $_SERVER['SERVER_PORT'] : 80;
+
+        if (($scheme === 'http' && $port === 80)
+            || ($scheme === 'https' && $port === 443)
         ) {
             return $name;
         }
 
-        return $name . ':' . $port;
+        return sprintf('%s:%d', $name, $port);
     }
 
     /**
@@ -304,4 +307,38 @@ protected function _getRawBody()
 
         return strlen(trim($body)) > 0 ? $body : false;
     }
+
+    /**
+     * Build the callback URL from the REQUEST_URI server parameter.
+     *
+     * @return string
+     */
+    private function buildCallbackUrlFromRequestUri()
+    {
+        $callbackUrl = $_SERVER['REQUEST_URI'];
+        $https = isset($_SERVER['HTTPS']) ? $_SERVER['HTTPS'] : null;
+        $scheme = $https === 'on' ? 'https' : 'http';
+        if ($https === 'on') {
+            $scheme = 'https';
+        }
+        $schemeAndHttpHost = $scheme . '://' . $this->_getHttpHost();
+        if (strpos($callbackUrl, $schemeAndHttpHost) === 0) {
+            $callbackUrl = substr($callbackUrl, strlen($schemeAndHttpHost));
+        }
+        return $callbackUrl;
+    }
+
+    /**
+     * Build the callback URL from the ORIG_PATH_INFO server parameter.
+     *
+     * @return string
+     */
+    private function buildCallbackUrlFromOrigPathInfo()
+    {
+        $callbackUrl = $_SERVER['ORIG_PATH_INFO'];
+        if (! empty($_SERVER['QUERY_STRING'])) {
+            $callbackUrl .= '?' . $_SERVER['QUERY_STRING'];
+        }
+        return $callbackUrl;
+    }
 }
diff --git a/test/PubSubHubbub/AbstractCallbackTest.php b/test/PubSubHubbub/AbstractCallbackTest.php
@@ -0,0 +1,85 @@
+<?php
+/**
+ * @see       https://github.com/zendframework/zend-feed for the canonical source repository
+ * @copyright Copyright (c) 2018 Zend Technologies USA Inc. (https://www.zend.com)
+ * @license   https://github.com/zendframework/zend-feed/blob/master/LICENSE.md New BSD License
+ */
+
+namespace ZendTest\Feed\PubSubHubbub;
+
+use PHPUnit\Framework\TestCase;
+use ReflectionMethod;
+use Zend\Feed\PubSubHubbub\AbstractCallback;
+
+class AbstractCallbackTest extends TestCase
+{
+    public function testDetectCallbackUrlIgnoresXOriginalUrlHeaderWhenXRewriteUrlHeaderIsNotPresent()
+    {
+        $_SERVER = array_merge($_SERVER, [
+            'HTTP_X_ORIGINAL_URL' => '/hijack-attempt',
+            'HTTPS' => 'on',
+            'HTTP_HOST' => 'example.com',
+            'REQUEST_URI' => '/requested/path',
+        ]);
+
+        $callback = new TestAsset\Callback();
+
+        $r = new ReflectionMethod($callback, '_detectCallbackUrl');
+        $r->setAccessible(true);
+
+        $this->assertSame('/requested/path', $r->invoke($callback));
+    }
+
+    public function testDetectCallbackUrlRequiresCombinationOfIISWasUrlRewrittenAndUnencodedUrlToReturnEarly()
+    {
+        $_SERVER = array_merge($_SERVER, [
+            'IIS_WasUrlRewritten' => '1',
+            'UNENCODED_URL' => '/requested/path',
+        ]);
+
+        $callback = new TestAsset\Callback();
+
+        $r = new ReflectionMethod($callback, '_detectCallbackUrl');
+        $r->setAccessible(true);
+
+        $this->assertSame('/requested/path', $r->invoke($callback));
+    }
+
+    public function testDetectCallbackUrlUsesRequestUriWhenNoOtherRewriteHeadersAreFound()
+    {
+        $_SERVER = array_merge($_SERVER, [
+            'REQUEST_URI' => '/expected/path'
+        ]);
+
+        $callback = new TestAsset\Callback();
+
+        $r = new ReflectionMethod($callback, '_detectCallbackUrl');
+        $r->setAccessible(true);
+
+        $this->assertSame('/expected/path', $r->invoke($callback));
+    }
+
+    public function testDetectCallbackUrlFallsBackToOrigPathInfoWhenAllElseFails()
+    {
+        $_SERVER = array_merge($_SERVER, [
+            'ORIG_PATH_INFO' => '/expected/path',
+        ]);
+
+        $callback = new TestAsset\Callback();
+
+        $r = new ReflectionMethod($callback, '_detectCallbackUrl');
+        $r->setAccessible(true);
+
+        $this->assertSame('/expected/path', $r->invoke($callback));
+    }
+
+    public function testDetectCallbackReturnsEmptyStringIfNoResourcesMatchedInServerSuperglobal()
+    {
+        $callback = new TestAsset\Callback();
+
+        $r = new ReflectionMethod($callback, '_detectCallbackUrl');
+        $r->setAccessible(true);
+
+        $this->assertSame('', $r->invoke($callback));
+    }
+}
diff --git a/test/PubSubHubbub/TestAsset/Callback.php b/test/PubSubHubbub/TestAsset/Callback.php
@@ -0,0 +1,21 @@
+<?php
+/**
+ * @see       https://github.com/zendframework/zend-feed for the canonical source repository
+ * @copyright Copyright (c) 2018 Zend Technologies USA Inc. (https://www.zend.com)
+ * @license   https://github.com/zendframework/zend-feed/blob/master/LICENSE.md New BSD License
+ */
+
+namespace ZendTest\Feed\PubSubHubbub\TestAsset;
+
+use Zend\Feed\PubSubHubbub\AbstractCallback;
+
+class Callback extends AbstractCallback
+{
+    /**
+     * {@inheritDoc}
+     */
+    public function handle(array $httpData = null, $sendResponseNow = false)
+    {
+        return false;
+    }
+}