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

Changes from 1 commit
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
Next Next commit
Prevent creating users with existing files 
Signed-off-by: Joas Schilling <[email protected]>
  • Loading branch information
@nickvergessen
nickvergessen committed Dec 4, 2019
commit dd53fad898e8b1de75efeda094b2f0f037d8a407
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);
}
}