Skip to content

Commit

Permalink
Validate if the user part of a "cloud id" can even be a valid user id
Browse files Browse the repository at this point in the history
Signed-off-by: Joas Schilling <[email protected]>
  • Loading branch information
nickvergessen committed Dec 9, 2022
1 parent 2f7a1fc commit 256fbe9
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 26 deletions.
3 changes: 3 additions & 0 deletions lib/private/Federation/CloudIdManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,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));
}
Expand Down
70 changes: 44 additions & 26 deletions lib/private/User/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -427,31 +429,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) === '') {
Expand Down Expand Up @@ -726,7 +704,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, [
Expand All @@ -740,6 +754,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);
Expand Down
8 changes: 8 additions & 0 deletions lib/public/IUserManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -212,4 +212,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;
}

0 comments on commit 256fbe9

Please sign in to comment.