From 5fb797a947bcb765bb09ab26a5bc7257ee3eabd1 Mon Sep 17 00:00:00 2001 From: Johannes Ernst Date: Tue, 5 Jul 2016 18:49:18 +0000 Subject: [PATCH 1/5] Allow wildcard * to be used in trusted domains, to support setups where no reliable DNS entry is available (e.g. mDNS) or for simple-to-setup aliasing (e.g. *.example.com) --- lib/private/Security/TrustedDomainHelper.php | 32 ++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/private/Security/TrustedDomainHelper.php b/lib/private/Security/TrustedDomainHelper.php index 75407ae39390a..6afefcbbe694d 100644 --- a/lib/private/Security/TrustedDomainHelper.php +++ b/lib/private/Security/TrustedDomainHelper.php @@ -84,7 +84,35 @@ public function isTrustedDomain($domainWithPort) { return true; } - return in_array($domain, $trustedList, true); - } + if(in_array($domain, $trustedList, true)) { + return true; + } + // If a value contains a *, apply glob-style matching. Any second * is ignored. + foreach ($trustedList as $trusted) { + if($trusted == '*') { + return true; + } + $star = strpos($trusted, '*'); + if($star === false) { + next; + } + if($star === 0) { + if(strrpos($domain, substr($trusted, 1)) !== false) { + return true; + } + } elseif($star === strlen($trusted)-1) { + if(strpos($domain, substr($trusted, 0, strlen($trusted)-1 )) !== false) { + return true; + } + } else { + if(strpos($domain, substr($trusted, 0, $star)) !== false + && strrpos($domain, substr($trusted, $star+1 ), -strlen($trusted-$star-1)) !== false ) + { + return true; + } + } + } + return false; + } } From c549b278177d76fe2b46cf0baaa19d28beb74a1e Mon Sep 17 00:00:00 2001 From: Johannes Ernst Date: Wed, 6 Jul 2016 04:51:49 +0000 Subject: [PATCH 2/5] Duh, no 'next' in PHP. Use === instead of == for extra paranoia. --- lib/private/Security/TrustedDomainHelper.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/Security/TrustedDomainHelper.php b/lib/private/Security/TrustedDomainHelper.php index 6afefcbbe694d..be4014327b3a1 100644 --- a/lib/private/Security/TrustedDomainHelper.php +++ b/lib/private/Security/TrustedDomainHelper.php @@ -90,12 +90,12 @@ public function isTrustedDomain($domainWithPort) { // If a value contains a *, apply glob-style matching. Any second * is ignored. foreach ($trustedList as $trusted) { - if($trusted == '*') { + if($trusted === '*') { return true; } $star = strpos($trusted, '*'); if($star === false) { - next; + break; } if($star === 0) { if(strrpos($domain, substr($trusted, 1)) !== false) { From 20ebc2a284d3862bce4021914f7e171e530cf857 Mon Sep 17 00:00:00 2001 From: Johannes Ernst Date: Wed, 6 Jul 2016 23:38:30 +0000 Subject: [PATCH 3/5] Trusted domain wildcard checking made shorter, supporting multiple * Added test cases --- lib/private/Security/TrustedDomainHelper.php | 40 ++++--------------- .../lib/Security/TrustedDomainHelperTest.php | 27 ++++++++++++- 2 files changed, 34 insertions(+), 33 deletions(-) diff --git a/lib/private/Security/TrustedDomainHelper.php b/lib/private/Security/TrustedDomainHelper.php index be4014327b3a1..44e133746fd11 100644 --- a/lib/private/Security/TrustedDomainHelper.php +++ b/lib/private/Security/TrustedDomainHelper.php @@ -70,7 +70,7 @@ public function isTrustedDomain($domainWithPort) { // Read trusted domains from config $trustedList = $this->config->getSystemValue('trusted_domains', []); - if(!is_array($trustedList)) { + if (!is_array($trustedList)) { return false; } @@ -79,39 +79,15 @@ public function isTrustedDomain($domainWithPort) { return true; } - // Compare with port appended - if(in_array($domainWithPort, $trustedList, true)) { - return true; - } - - if(in_array($domain, $trustedList, true)) { - return true; - } - - // If a value contains a *, apply glob-style matching. Any second * is ignored. - foreach ($trustedList as $trusted) { - if($trusted === '*') { + // match, allowing for * wildcards + foreach ($trustedList as $trusted) { + if (gettype($trusted) !== 'string') { + break; + } + $regex = '/^' . join('.*', array_map(function($v) { return preg_quote($v, '/'); }, explode('*', $trusted))) . '$/'; + if (preg_match($regex, $domain) || preg_match($regex, $domainWithPort)) { return true; } - $star = strpos($trusted, '*'); - if($star === false) { - break; - } - if($star === 0) { - if(strrpos($domain, substr($trusted, 1)) !== false) { - return true; - } - } elseif($star === strlen($trusted)-1) { - if(strpos($domain, substr($trusted, 0, strlen($trusted)-1 )) !== false) { - return true; - } - } else { - if(strpos($domain, substr($trusted, 0, $star)) !== false - && strrpos($domain, substr($trusted, $star+1 ), -strlen($trusted-$star-1)) !== false ) - { - return true; - } - } } return false; } diff --git a/tests/lib/Security/TrustedDomainHelperTest.php b/tests/lib/Security/TrustedDomainHelperTest.php index dfd51167ccaa6..6c254dcaa79a8 100644 --- a/tests/lib/Security/TrustedDomainHelperTest.php +++ b/tests/lib/Security/TrustedDomainHelperTest.php @@ -49,6 +49,11 @@ public function trustedDomainDataProvider() { 'host.two.test', '[1fff:0:a88:85a3::ac1f]', 'host.three.test:443', + '*.leading.host', + 'trailing.host*', + 'cen*ter', + '*.leadingwith.port:123', + 'trailingwith.port*:456', ]; return [ // empty defaults to false with 8.1 @@ -76,7 +81,27 @@ public function trustedDomainDataProvider() { [$trustedHostTestList, 'localhost: evil.host', false], // do not trust casting [[1], '1', false], + // leading * + [$trustedHostTestList, 'abc.leading.host', true], + [$trustedHostTestList, 'abc.def.leading.host', true], + [$trustedHostTestList, 'abc.def.leading.host.another', false], + [$trustedHostTestList, 'abc.def.leading.host:123', true], + [$trustedHostTestList, 'leading.host', false], + // trailing * + [$trustedHostTestList, 'trailing.host', true], + [$trustedHostTestList, 'trailing.host.abc', true], + [$trustedHostTestList, 'trailing.host.abc.def', true], + [$trustedHostTestList, 'trailing.host.abc:123', true], + [$trustedHostTestList, 'another.trailing.host', false], + // center * + [$trustedHostTestList, 'center', true], + [$trustedHostTestList, 'cenxxxter', true], + [$trustedHostTestList, 'cen.x.y.ter', true], + // with port + [$trustedHostTestList, 'abc.leadingwith.port:123', true], + [$trustedHostTestList, 'abc.leadingwith.port:1234', false], + [$trustedHostTestList, 'trailingwith.port.abc:456', true], + [$trustedHostTestList, 'trailingwith.port.abc:123', false], ]; } - } From d6e5c4cbdce9387d924819826132f17781454bf9 Mon Sep 17 00:00:00 2001 From: Johannes Ernst Date: Wed, 6 Jul 2016 23:51:04 +0000 Subject: [PATCH 4/5] Disallow certain malformed domain names even if they match the trusted domain expression Stricter checking for valid domain names --- lib/private/Security/TrustedDomainHelper.php | 9 ++++++--- tests/lib/Security/TrustedDomainHelperTest.php | 4 ++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/private/Security/TrustedDomainHelper.php b/lib/private/Security/TrustedDomainHelper.php index 44e133746fd11..cf4def63dd3e1 100644 --- a/lib/private/Security/TrustedDomainHelper.php +++ b/lib/private/Security/TrustedDomainHelper.php @@ -78,13 +78,16 @@ public function isTrustedDomain($domainWithPort) { if (preg_match(Request::REGEX_LOCALHOST, $domain) === 1) { return true; } - - // match, allowing for * wildcards + // Reject misformed domains in any case + if (strpos($domain,'-') === 0 || strpos($domain,'..') !== false) { + return false; + } + // Match, allowing for * wildcards foreach ($trustedList as $trusted) { if (gettype($trusted) !== 'string') { break; } - $regex = '/^' . join('.*', array_map(function($v) { return preg_quote($v, '/'); }, explode('*', $trusted))) . '$/'; + $regex = '/^' . join('[-\.a-zA-Z0-9]*', array_map(function($v) { return preg_quote($v, '/'); }, explode('*', $trusted))) . '$/'; if (preg_match($regex, $domain) || preg_match($regex, $domainWithPort)) { return true; } diff --git a/tests/lib/Security/TrustedDomainHelperTest.php b/tests/lib/Security/TrustedDomainHelperTest.php index 6c254dcaa79a8..1beb7a6671795 100644 --- a/tests/lib/Security/TrustedDomainHelperTest.php +++ b/tests/lib/Security/TrustedDomainHelperTest.php @@ -102,6 +102,10 @@ public function trustedDomainDataProvider() { [$trustedHostTestList, 'abc.leadingwith.port:1234', false], [$trustedHostTestList, 'trailingwith.port.abc:456', true], [$trustedHostTestList, 'trailingwith.port.abc:123', false], + // bad hostname + [$trustedHostTestList, '-bad', false], + [$trustedHostTestList, '-bad.leading.host', false], + [$trustedHostTestList, 'bad..der.leading.host', false], ]; } } From 2b96e904d092f6e44be7df1380fbaf64821b2b50 Mon Sep 17 00:00:00 2001 From: Johannes Ernst Date: Thu, 7 Jul 2016 16:23:20 +0000 Subject: [PATCH 5/5] Extended documentation on trusted_domains to cover ports and wildcards. --- config/config.sample.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/config/config.sample.php b/config/config.sample.php index 6285e096ba799..14bc4eac890f4 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -58,6 +58,12 @@ * Your list of trusted domains that users can log into. Specifying trusted * domains prevents host header poisoning. Do not remove this, as it performs * necessary security checks. + * You can specify: + * - the exact hostname of your host or virtual host, e.g. demo.example.org. + * - the exact hostname with permitted port, e.g. demo.example.org:443. + * This disallows all other ports on this host + * - use * as a wildcard, e.g. ubos-raspberry-pi*.local will allow + * ubos-raspberry-pi.local and ubos-raspberry-pi-2.local */ 'trusted_domains' => array (