From: Joas Schilling <coding@schilljs.com>
Date: Wed, 12 Apr 2017 08:29:28 +0000 (+0200)
Subject: Better validation of allowed user names
X-Git-Tag: v12.0.0beta1~133^2
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=refs%2Fpull%2F4371%2Fhead;p=nextcloud-server.git

Better validation of allowed user names

Signed-off-by: Joas Schilling <coding@schilljs.com>
---

diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php
index b62b04febaf..6220613cbb1 100644
--- a/lib/private/User/Manager.php
+++ b/lib/private/User/Manager.php
@@ -295,9 +295,13 @@ class Manager extends PublicEmitter implements IUserManager {
 			throw new \Exception($l->t('A valid username must be provided'));
 		}
 		// No whitespace at the beginning or at the end
-		if (strlen(trim($uid, "\t\n\r\0\x0B\xe2\x80\x8b")) !== strlen(trim($uid))) {
+		if (trim($uid) !== $uid) {
 			throw new \Exception($l->t('Username contains whitespace at the beginning or at the end'));
 		}
+		// Username only consists of 1 or 2 dots (directory traversal)
+		if ($uid === '.' || $uid === '..') {
+			throw new \Exception($l->t('Username must not consist of dots only'));
+		}
 		// No empty password
 		if (trim($password) == '') {
 			throw new \Exception($l->t('A valid password must be provided'));
diff --git a/tests/lib/User/ManagerTest.php b/tests/lib/User/ManagerTest.php
index 123271bcc8f..671b2ac57c1 100644
--- a/tests/lib/User/ManagerTest.php
+++ b/tests/lib/User/ManagerTest.php
@@ -256,6 +256,49 @@ class ManagerTest extends TestCase {
 		$this->assertEquals('foo3', array_shift($result)->getUID());
 	}
 
+	public function dataCreateUserInvalid() {
+		return [
+			['te?st', 'foo', 'Only the following characters are allowed in a username:'
+				. ' "a-z", "A-Z", "0-9", and "_.@-\'"'],
+			["te\tst", '', 'Only the following characters are allowed in a username:'
+				. ' "a-z", "A-Z", "0-9", and "_.@-\'"'],
+			["te\nst", '', 'Only the following characters are allowed in a username:'
+				. ' "a-z", "A-Z", "0-9", and "_.@-\'"'],
+			["te\rst", '', 'Only the following characters are allowed in a username:'
+				. ' "a-z", "A-Z", "0-9", and "_.@-\'"'],
+			["te\0st", '', 'Only the following characters are allowed in a username:'
+				. ' "a-z", "A-Z", "0-9", and "_.@-\'"'],
+			["te\x0Bst", '', 'Only the following characters are allowed in a username:'
+				. ' "a-z", "A-Z", "0-9", and "_.@-\'"'],
+			["te\xe2st", '', 'Only the following characters are allowed in a username:'
+				. ' "a-z", "A-Z", "0-9", and "_.@-\'"'],
+			["te\x80st", '', 'Only the following characters are allowed in a username:'
+				. ' "a-z", "A-Z", "0-9", and "_.@-\'"'],
+			["te\x8bst", '', 'Only the following characters are allowed in a username:'
+				. ' "a-z", "A-Z", "0-9", and "_.@-\'"'],
+			['', 'foo', 'A valid username must be provided'],
+			[' ', 'foo', 'A valid username must be provided'],
+			[' test', 'foo', 'Username contains whitespace at the beginning or at the end'],
+			['test ', 'foo', 'Username contains whitespace at the beginning or at the end'],
+			['.', 'foo', 'Username must not consist of dots only'],
+			['..', 'foo', 'Username must not consist of dots only'],
+			['.test', '', 'A valid password must be provided'],
+			['test', '', 'A valid password must be provided'],
+		];
+	}
+
+	/**
+	 * @dataProvider dataCreateUserInvalid
+	 */
+	public function testCreateUserInvalid($uid, $password, $exception) {
+
+		$this->setExpectedException(\Exception::class, $exception);
+
+		$manager = new \OC\User\Manager($this->config);
+		$manager->createUser($uid, $password);
+
+	}
+
 	public function testCreateUserSingleBackendNotExists() {
 		/**
 		 * @var \Test\Util\User\Dummy | \PHPUnit_Framework_MockObject_MockObject $backend