From 421cff00bdb6441b1639c83ed68dc1cbe196e333 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 5 Mar 2014 15:02:05 +0100 Subject: [PATCH 1/4] Show warning page when accessing server from an untrusted domain Added early check for the requested domain host and show a warning page if the domain is not trusted. --- lib/base.php | 16 ++++++++ lib/private/request.php | 82 ++++++++++++++++++++++++++++++----------- 2 files changed, 76 insertions(+), 22 deletions(-) diff --git a/lib/base.php b/lib/base.php index 351b91b7dfa..82612a18771 100644 --- a/lib/base.php +++ b/lib/base.php @@ -694,6 +694,22 @@ class OC { exit(); } + $host = OC_Request::insecureServerHost(); + // if the host passed in headers isn't trusted + if (!OC::$CLI + // overwritehost is always trusted + && OC_Request::getOverwriteHost() === null + && !OC_Request::isTrustedDomain($host)) { + + header('HTTP/1.1 400 Bad Request'); + header('Status: 400 Bad Request'); + OC_Template::printErrorPage( + 'You are accessing the server from an untrusted domain.', + 'Please contact your administrator' + ); + return; + } + $request = OC_Request::getPathInfo(); if (substr($request, -3) !== '.js') { // we need these files during the upgrade self::checkMaintenanceMode(); diff --git a/lib/private/request.php b/lib/private/request.php index afd3fda4f2d..fb387e83e3a 100755 --- a/lib/private/request.php +++ b/lib/private/request.php @@ -25,17 +25,64 @@ class OC_Request { } /** - * @brief Checks whether a domain is considered as trusted. This is used to prevent Host Header Poisoning. + * @brief Checks whether a domain is considered as trusted from the list + * of trusted domains. If no trusted domains have been configured, returns + * true. + * This is used to prevent Host Header Poisoning. * @param string $host - * @return bool + * @return bool true if the given domain is trusted or if no trusted domains + * have been configured */ public static function isTrustedDomain($domain) { - $trustedList = \OC_Config::getValue('trusted_domains', array('')); + $trustedList = \OC_Config::getValue('trusted_domains', array()); + if (empty($trustedList)) { + return true; + } return in_array($domain, $trustedList); } /** - * @brief Returns the server host + * @brief Returns the unverified server host from the headers without checking + * whether it is a trusted domain + * @returns string the server host + * + * Returns the server host, even if the website uses one or more + * reverse proxies + */ + public static function insecureServerHost() { + $host = null; + if (isset($_SERVER['HTTP_X_FORWARDED_HOST'])) { + if (strpos($_SERVER['HTTP_X_FORWARDED_HOST'], ",") !== false) { + $host = trim(array_pop(explode(",", $_SERVER['HTTP_X_FORWARDED_HOST']))); + } else { + $host = $_SERVER['HTTP_X_FORWARDED_HOST']; + } + } else { + if (isset($_SERVER['HTTP_HOST'])) { + $host = $_SERVER['HTTP_HOST']; + } else if (isset($_SERVER['SERVER_NAME'])) { + $host = $_SERVER['SERVER_NAME']; + } + } + return $host; + } + + /** + * Returns the overwritehost setting from the config if set and + * if the overwrite condition is met + * @return overwritehost value or null if not defined or the defined condition + * isn't met + */ + public static function getOverwriteHost() { + if(OC_Config::getValue('overwritehost', '') !== '' and self::isOverwriteCondition()) { + return OC_Config::getValue('overwritehost'); + } + return null; + } + + /** + * @brief Returns the server host from the headers, or the first configured + * trusted domain if the host isn't in the trusted list * @returns string the server host * * Returns the server host, even if the website uses one or more @@ -45,29 +92,20 @@ class OC_Request { if(OC::$CLI) { return 'localhost'; } - if(OC_Config::getValue('overwritehost', '') !== '' and self::isOverwriteCondition()) { - return OC_Config::getValue('overwritehost'); - } - if (isset($_SERVER['HTTP_X_FORWARDED_HOST'])) { - if (strpos($_SERVER['HTTP_X_FORWARDED_HOST'], ",") !== false) { - $host = trim(array_pop(explode(",", $_SERVER['HTTP_X_FORWARDED_HOST']))); - } - else{ - $host = $_SERVER['HTTP_X_FORWARDED_HOST']; - } - } else { - if (isset($_SERVER['HTTP_HOST'])) { - $host = $_SERVER['HTTP_HOST']; - } - else if (isset($_SERVER['SERVER_NAME'])) { - $host = $_SERVER['SERVER_NAME']; - } + + // overwritehost is always trusted + $host = self::getOverwriteHost(); + if ($host !== null) { + return $host; } + // get the host from the headers + $host = self::insecureServerHost(); + // Verify that the host is a trusted domain if the trusted domains // are defined // If no trusted domain is provided the first trusted domain is returned - if(self::isTrustedDomain($host) || \OC_Config::getValue('trusted_domains', "") === "") { + if (self::isTrustedDomain($host)) { return $host; } else { $trustedList = \OC_Config::getValue('trusted_domains', array('')); From f8fe2753b125c6882019d1ba2c2db661b0bdcbf2 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 5 Mar 2014 15:41:28 +0100 Subject: [PATCH 2/4] Added localhost as trusted domain --- lib/private/request.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/private/request.php b/lib/private/request.php index fb387e83e3a..347d77b3724 100755 --- a/lib/private/request.php +++ b/lib/private/request.php @@ -13,6 +13,8 @@ class OC_Request { const USER_AGENT_ANDROID_MOBILE_CHROME = '#Android.*Chrome/[.0-9]*#'; const USER_AGENT_FREEBOX = '#^Mozilla/5\.0$#'; + const REGEX_LOCALHOST = '/^(127\.0\.0\.1|localhost)(:[0-9]+|)$/'; + /** * @brief Check overwrite condition * @param string $type @@ -38,6 +40,9 @@ class OC_Request { if (empty($trustedList)) { return true; } + if (preg_match(self::REGEX_LOCALHOST, $domain) === 1) { + return true; + } return in_array($domain, $trustedList); } From c7e204bd3674b616faddbb0b88187f94638e5b01 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 5 Mar 2014 17:04:15 +0100 Subject: [PATCH 3/4] Added unit tests for serverHost and other related functions --- tests/lib/request.php | 137 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/tests/lib/request.php b/tests/lib/request.php index 1d77acc70ae..bff84e1b03f 100644 --- a/tests/lib/request.php +++ b/tests/lib/request.php @@ -135,4 +135,141 @@ class Test_Request extends PHPUnit_Framework_TestCase { ), ); } + + public function testInsecureServerHost() { + unset($_SERVER['HTTP_X_FORWARDED_HOST']); + unset($_SERVER['HTTP_HOST']); + unset($_SERVER['SERVER_NAME']); + $_SERVER['SERVER_NAME'] = 'from.server.name:8080'; + $host = OC_Request::insecureServerHost(); + $this->assertEquals('from.server.name:8080', $host); + + $_SERVER['HTTP_HOST'] = 'from.host.header:8080'; + $host = OC_Request::insecureServerHost(); + $this->assertEquals('from.host.header:8080', $host); + + $_SERVER['HTTP_X_FORWARDED_HOST'] = 'from.forwarded.host:8080'; + $host = OC_Request::insecureServerHost(); + $this->assertEquals('from.forwarded.host:8080', $host); + + $_SERVER['HTTP_X_FORWARDED_HOST'] = 'from.forwarded.host2:8080,another.one:9000'; + $host = OC_Request::insecureServerHost(); + $this->assertEquals('from.forwarded.host2:8080', $host); + + // clean up + unset($_SERVER['HTTP_X_FORWARDED_HOST']); + unset($_SERVER['HTTP_HOST']); + unset($_SERVER['SERVER_NAME']); + } + + public function testGetOverwriteHost() { + unset($_SERVER['REMOTE_ADDR']); + OC_Config::deleteKey('overwritecondaddr'); + OC_Config::deleteKey('overwritehost'); + $host = OC_Request::getOverwriteHost(); + $this->assertNull($host); + + OC_Config::setValue('overwritehost', ''); + $host = OC_Request::getOverwriteHost(); + $this->assertNull($host); + + OC_Config::setValue('overwritehost', 'host.one.test:8080'); + $host = OC_Request::getOverwriteHost(); + $this->assertEquals('host.one.test:8080', $host); + + $_SERVER['REMOTE_ADDR'] = 'somehost.test:8080'; + OC_Config::setValue('overwritecondaddr', '^somehost\..*$'); + $host = OC_Request::getOverwriteHost(); + $this->assertEquals('host.one.test:8080', $host); + + OC_Config::setValue('overwritecondaddr', '^somethingelse.*$'); + $host = OC_Request::getOverwriteHost(); + $this->assertNull($host); + + // clean up + unset($_SERVER['REMOTE_ADDR']); + OC_Config::deleteKey('overwritecondaddr'); + OC_Config::deleteKey('overwritehost'); + } + + /** + * @dataProvider trustedDomainDataProvider + */ + public function testIsTrustedDomain($trustedDomains, $testDomain, $result) { + OC_Config::deleteKey('trusted_domains'); + if ($trustedDomains !== null) { + OC_Config::setValue('trusted_domains', $trustedDomains); + } + + $this->assertEquals($result, OC_Request::isTrustedDomain($testDomain)); + + // clean up + OC_Config::deleteKey('trusted_domains'); + } + + public function trustedDomainDataProvider() { + $trustedHostTestList = array('host.one.test:8080', 'host.two.test:8080'); + return array( + // empty defaults to true + array(null, 'host.one.test:8080', true), + array('', 'host.one.test:8080', true), + array(array(), 'host.one.test:8080', true), + + // trust list when defined + array($trustedHostTestList, 'host.two.test:8080', true), + array($trustedHostTestList, 'host.two.test:9999', false), + array($trustedHostTestList, 'host.three.test:8080', false), + + // trust localhost regardless of trust list + array($trustedHostTestList, 'localhost', true), + array($trustedHostTestList, 'localhost:8080', true), + array($trustedHostTestList, '127.0.0.1', true), + array($trustedHostTestList, '127.0.0.1:8080', true), + + // do not trust invalid localhosts + array($trustedHostTestList, 'localhost:1:2', false), + array($trustedHostTestList, 'localhost: evil.host', false), + ); + } + + public function testServerHost() { + OC_Config::deleteKey('overwritecondaddr'); + OC_Config::setValue('overwritehost', 'overwritten.host:8080'); + OC_Config::setValue( + 'trusted_domains', + array( + 'trusted.host:8080', + 'second.trusted.host:8080' + ) + ); + $_SERVER['HTTP_HOST'] = 'trusted.host:8080'; + + // CLI always gives localhost + $oldCLI = OC::$CLI; + OC::$CLI = true; + $host = OC_Request::serverHost(); + $this->assertEquals('localhost', $host); + OC::$CLI = false; + + // overwritehost overrides trusted domain + $host = OC_Request::serverHost(); + $this->assertEquals('overwritten.host:8080', $host); + + // trusted domain returned when used + OC_Config::deleteKey('overwritehost'); + $host = OC_Request::serverHost(); + $this->assertEquals('trusted.host:8080', $host); + + // trusted domain returned when untrusted one in header + $_SERVER['HTTP_HOST'] = 'untrusted.host:8080'; + OC_Config::deleteKey('overwritehost'); + $host = OC_Request::serverHost(); + $this->assertEquals('trusted.host:8080', $host); + + // clean up + OC_Config::deleteKey('overwritecondaddr'); + OC_Config::deleteKey('overwritehost'); + unset($_SERVER['HTTP_HOST']); + OC::$CLI = $oldCLI; + } } From 9136e6ad3028040b91685fc94e8fccd29c9b9210 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Wed, 5 Mar 2014 17:04:45 +0100 Subject: [PATCH 4/4] Fixed X-Forwarded-Host parsing --- lib/private/request.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/private/request.php b/lib/private/request.php index 347d77b3724..8041c4f0048 100755 --- a/lib/private/request.php +++ b/lib/private/request.php @@ -58,7 +58,8 @@ class OC_Request { $host = null; if (isset($_SERVER['HTTP_X_FORWARDED_HOST'])) { if (strpos($_SERVER['HTTP_X_FORWARDED_HOST'], ",") !== false) { - $host = trim(array_pop(explode(",", $_SERVER['HTTP_X_FORWARDED_HOST']))); + $parts = explode(',', $_SERVER['HTTP_X_FORWARDED_HOST']); + $host = trim(current($parts)); } else { $host = $_SERVER['HTTP_X_FORWARDED_HOST']; }