Merge pull request #18924 from nextcloud/fix/18848/array-index

Make getServerHost more robust to faulty user input

lib/private/AppFramework/Http/Request.php

@@ -904,14 +904,14 @@ class Request implements \ArrayAccess, \Countable, IRequest {
 		$trustedDomainHelper = new TrustedDomainHelper($this->config);
 		if ($trustedDomainHelper->isTrustedDomain($host)) {
 			return $host;
-		} else {
-			$trustedList = $this->config->getSystemValue('trusted_domains', []);
-			if(!empty($trustedList)) {
-				return $trustedList[0];
-			} else {
-				return '';
-			}
 		}
+
+		$trustedList = (array)$this->config->getSystemValue('trusted_domains', []);
+		if (count($trustedList) > 0) {
+			return reset($trustedList);
+		}
+
+		return '';
 	}
 
 	/**

tests/lib/AppFramework/Http/RequestTest.php

@@ -1222,6 +1222,52 @@ class RequestTest extends \Test\TestCase {
 		$this->assertSame('',  $request->getServerHost());
 	}
 
+	/**
+	 * @return array
+	 */
+	public function dataGetServerHostTrustedDomain() {
+		return [
+			'is array' => ['my.trusted.host', ['my.trusted.host']],
+			'is array but undefined index 0' => ['my.trusted.host', [2 => 'my.trusted.host']],
+			'is string' => ['my.trusted.host', 'my.trusted.host'],
+			'is null' => ['', null],
+		];
+	}
+
+	/**
+	 * @dataProvider dataGetServerHostTrustedDomain
+	 * @param $expected
+	 * @param $trustedDomain
+	 */
+	public function testGetServerHostTrustedDomain($expected, $trustedDomain) {
+		$this->config
+			->method('getSystemValue')
+			->willReturnCallback(function ($key, $default) use ($trustedDomain) {
+				if ($key === 'trusted_proxies') {
+					return ['1.2.3.4'];
+				}
+				if ($key === 'trusted_domains') {
+					return $trustedDomain;
+				}
+				return $default;
+			});
+
+		$request = new Request(
+			[
+				'server' => [
+					'HTTP_X_FORWARDED_HOST' => 'my.untrusted.host',
+					'REMOTE_ADDR' => '1.2.3.4',
+				],
+			],
+			$this->secureRandom,
+			$this->config,
+			$this->csrfTokenManager,
+			$this->stream
+		);
+
+		$this->assertSame($expected, $request->getServerHost());
+	}
+
 	public function testGetOverwriteHostDefaultNull() {
 		$this->config
 			->expects($this->once())