From e78656f3720c7fc1b6ec7a40e9e01ab440fb26b4 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 11 Nov 2022 09:37:26 +0100 Subject: [PATCH 1/3] Validate if the user part of a "cloud id" can even be a valid user id Signed-off-by: Joas Schilling --- lib/private/Federation/CloudIdManager.php | 3 + lib/private/User/Manager.php | 70 ++++++++++++++--------- lib/public/IUserManager.php | 8 +++ 3 files changed, 55 insertions(+), 26 deletions(-) diff --git a/lib/private/Federation/CloudIdManager.php b/lib/private/Federation/CloudIdManager.php index 77bb9437ba290..5a857b97f253b 100644 --- a/lib/private/Federation/CloudIdManager.php +++ b/lib/private/Federation/CloudIdManager.php @@ -82,6 +82,9 @@ public function resolveCloudId(string $cloudId): ICloudId { if ($lastValidAtPos !== false) { $user = substr($id, 0, $lastValidAtPos); $remote = substr($id, $lastValidAtPos + 1); + + $this->userManager->validateUserId($user); + if (!empty($user) && !empty($remote)) { return new CloudId($id, $user, $remote, $this->getDisplayNameFromContact($id)); } diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 33744ad7ffe45..1bd25e48d0b91 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -44,6 +44,8 @@ use OCP\IUser; use OCP\IUserBackend; use OCP\IUserManager; +use OCP\L10N\IFactory; +use OCP\Server; use OCP\Support\Subscription\IAssertion; use OCP\User\Backend\IGetRealUIDBackend; use OCP\User\Backend\ISearchKnownUsersBackend; @@ -416,31 +418,7 @@ public function createUser($uid, $password) { public function createUserFromBackend($uid, $password, UserInterface $backend) { $l = \OC::$server->getL10N('lib'); - // Check the name for bad characters - // Allowed are: "a-z", "A-Z", "0-9" and "_.@-'" - if (preg_match('/[^a-zA-Z0-9 _.@\-\']/', $uid)) { - throw new \InvalidArgumentException($l->t('Only the following characters are allowed in a username:' - . ' "a-z", "A-Z", "0-9", and "_.@-\'"')); - } - - // No empty username - if (trim($uid) === '') { - throw new \InvalidArgumentException($l->t('A valid username must be provided')); - } - - // No whitespace at the beginning or at the end - if (trim($uid) !== $uid) { - throw new \InvalidArgumentException($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 \InvalidArgumentException($l->t('Username must not consist of dots only')); - } - - if (!$this->verifyUid($uid)) { - throw new \InvalidArgumentException($l->t('Username is invalid because files already exist for this user')); - } + $this->validateUserId($uid, true); // No empty password if (trim($password) === '') { @@ -717,7 +695,43 @@ public function getByEmail($email) { })); } - private function verifyUid(string $uid): bool { + /** + * @param string $uid + * @param bool $checkDataDirectory + * @throws \InvalidArgumentException Message is an already translated string with a reason why the id is not valid + * @since 26.0.0 + */ + public function validateUserId(string $uid, bool $checkDataDirectory = false): void { + $l = Server::get(IFactory::class)->get('lib'); + + // Check the name for bad characters + // Allowed are: "a-z", "A-Z", "0-9" and "_.@-'" + if (preg_match('/[^a-zA-Z0-9 _.@\-\']/', $uid)) { + throw new \InvalidArgumentException($l->t('Only the following characters are allowed in a username:' + . ' "a-z", "A-Z", "0-9", and "_.@-\'"')); + } + + // No empty username + if (trim($uid) === '') { + throw new \InvalidArgumentException($l->t('A valid username must be provided')); + } + + // No whitespace at the beginning or at the end + if (trim($uid) !== $uid) { + throw new \InvalidArgumentException($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 \InvalidArgumentException($l->t('Username must not consist of dots only')); + } + + if (!$this->verifyUid($uid, $checkDataDirectory)) { + throw new \InvalidArgumentException($l->t('Username is invalid because files already exist for this user')); + } + } + + private function verifyUid(string $uid, bool $checkDataDirectory = false): bool { $appdata = 'appdata_' . $this->config->getSystemValueString('instanceid'); if (\in_array($uid, [ @@ -731,6 +745,10 @@ private function verifyUid(string $uid): bool { return false; } + if (!$checkDataDirectory) { + return true; + } + $dataDirectory = $this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data'); return !file_exists(rtrim($dataDirectory, '/') . '/' . $uid); diff --git a/lib/public/IUserManager.php b/lib/public/IUserManager.php index e5c220af40c60..2771263bcfff9 100644 --- a/lib/public/IUserManager.php +++ b/lib/public/IUserManager.php @@ -203,4 +203,12 @@ public function callForSeenUsers(\Closure $callback); * @since 9.1.0 */ public function getByEmail($email); + + /** + * @param string $uid The user ID to validate + * @param bool $checkDataDirectory Whether it should be checked if files for the ID exist inside the data directory + * @throws \InvalidArgumentException Message is an already translated string with a reason why the ID is not valid + * @since 26.0.0 + */ + public function validateUserId(string $uid, bool $checkDataDirectory = false): void; } From 242c26ba9ee19017d18b7d556ab0afaa23344ebf Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 11 Nov 2022 09:44:24 +0100 Subject: [PATCH 2/3] Improve email search results Signed-off-by: Joas Schilling --- lib/private/Collaboration/Collaborators/MailPlugin.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/private/Collaboration/Collaborators/MailPlugin.php b/lib/private/Collaboration/Collaborators/MailPlugin.php index 8c2efce6f0d3b..d48bf2ade7972 100644 --- a/lib/private/Collaboration/Collaborators/MailPlugin.php +++ b/lib/private/Collaboration/Collaborators/MailPlugin.php @@ -101,6 +101,12 @@ public function search($search, $limit, $offset, ISearchResult $searchResult) { return false; } + // Extract the email address from "Foo Bar " and then search with "foo.bar@example.tld" instead + $result = preg_match('/<([^@]+@.+)>$/', $search, $matches); + if ($result && filter_var($matches[1], FILTER_VALIDATE_EMAIL)) { + return $this->search($matches[1], $limit, $offset, $searchResult); + } + $currentUserId = $this->userSession->getUser()->getUID(); $result = $userResults = ['wide' => [], 'exact' => []]; From 21e9fb82f901eb57979736be4e46adc37353a8a6 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 12 Dec 2022 09:40:53 +0100 Subject: [PATCH 3/3] Adjust API usage to Nextcloud 24 Signed-off-by: Joas Schilling --- lib/private/User/Manager.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/private/User/Manager.php b/lib/private/User/Manager.php index 1bd25e48d0b91..bd82236342158 100644 --- a/lib/private/User/Manager.php +++ b/lib/private/User/Manager.php @@ -45,7 +45,6 @@ use OCP\IUserBackend; use OCP\IUserManager; use OCP\L10N\IFactory; -use OCP\Server; use OCP\Support\Subscription\IAssertion; use OCP\User\Backend\IGetRealUIDBackend; use OCP\User\Backend\ISearchKnownUsersBackend; @@ -702,7 +701,7 @@ public function getByEmail($email) { * @since 26.0.0 */ public function validateUserId(string $uid, bool $checkDataDirectory = false): void { - $l = Server::get(IFactory::class)->get('lib'); + $l = \OC::$server->getL10N('lib'); // Check the name for bad characters // Allowed are: "a-z", "A-Z", "0-9" and "_.@-'"