Skip to content

Prevent creating users with existing files #18130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rullzer merged 2 commits into from
Dec 5, 2019
Merged

Prevent creating users with existing files #18130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd53fad
Prevent creating users with existing files
nickvergessen Nov 27, 2019
6004f62
I love unit tests that mock unnecessary stuff
nickvergessen Dec 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions lib/private/User/Manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,10 +294,6 @@ public function searchDisplayName($pattern, $limit = null, $offset = null) {
* @return bool|IUser the created user or false
*/
public function createUser($uid, $password) {
if (!$this->verifyUid($uid)) {
return false;
}

$localBackends = [];
foreach ($this->backends as $backend) {
if ($backend instanceof Database) {
Expand Down Expand Up @@ -332,22 +328,30 @@ public function createUserFromBackend($uid, $password, UserInterface $backend) {

// Check the name for bad characters
// Allowed are: "a-z", "A-Z", "0-9" and "_.@-'"
if (preg_match('/[^a-zA-Z0-9 _\.@\-\']/', $uid)) {
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'));
}

// No empty password
if (trim($password) === '') {
throw new \InvalidArgumentException($l->t('A valid password must be provided'));
Expand Down Expand Up @@ -623,10 +627,18 @@ public function getByEmail($email) {
private function verifyUid(string $uid): bool {
$appdata = 'appdata_' . $this->config->getSystemValueString('instanceid');

if ($uid === '.htaccess' || $uid === 'files_external' || $uid === '.ocdata' || $uid === 'owncloud.log' || $uid === 'nextcloud.log' || $uid === $appdata) {
if (\in_array($uid, [
'.htaccess',
'files_external',
'.ocdata',
'owncloud.log',
'nextcloud.log',
$appdata], true)) {
return false;
}

return true;
$dataDirectory = $this->config->getSystemValueString('datadirectory', \OC::$SERVERROOT . '/data');

return !file_exists(rtrim($dataDirectory, '/') . '/' . $uid);
}
}
9 changes: 9 additions & 0 deletions tests/lib/Cache/FileCacheTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,15 @@ protected function tearDown(): void {
\OC_User::setUserId($this->user);
\OC::$server->getConfig()->setSystemValue('cachedirectory', $this->datadir);

if ($this->instance) {
$this->instance->clear();
$this->instance = null;
}

//tear down the users dir aswell
$user = \OC::$server->getUserManager()->get('test');
$user->delete();

// Restore the original mount point
\OC\Files\Filesystem::clearMounts();
\OC\Files\Filesystem::mount($this->storage, array(), '/');
Expand Down