From c4cba43bade736ab89e31013a6ccee59a6e077ee Mon Sep 17 00:00:00 2001
From: Laurent Destailleur <eldy@destailleur.fr>
Date: Sun, 13 Jun 2021 16:15:05 +0200
Subject: [PATCH] FIX Broken Access Control reported by Ahsan Aziz.

---
 htdocs/langs/en_US/errors.lang   |  1 +
 htdocs/user/card.php             | 22 ++++++++++++++--------
 htdocs/user/class/user.class.php | 28 +++++++++++++++++++++++++++-
 3 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/htdocs/langs/en_US/errors.lang b/htdocs/langs/en_US/errors.lang
index f2d9106107b..c4e8a1d7226 100644
--- a/htdocs/langs/en_US/errors.lang
+++ b/htdocs/langs/en_US/errors.lang
@@ -11,6 +11,7 @@ ErrorBadValueForParamNotAString=Bad value for your parameter. It appends general
 ErrorRefAlreadyExists=Reference <b>%s</b> already exists.
 ErrorLoginAlreadyExists=Login %s already exists.
 ErrorGroupAlreadyExists=Group %s already exists.
+ErrorEmailAlreadyExists=Email %s already exists.
 ErrorRecordNotFound=Record not found.
 ErrorFailToCopyFile=Failed to copy file '<b>%s</b>' into '<b>%s</b>'.
 ErrorFailToCopyDir=Failed to copy directory '<b>%s</b>' into '<b>%s</b>'.
diff --git a/htdocs/user/card.php b/htdocs/user/card.php
index 0b67853b553..a78425bb596 100644
--- a/htdocs/user/card.php
+++ b/htdocs/user/card.php
@@ -444,16 +444,22 @@ if (empty($reshook)) {
 
 				$object->lang = GETPOST('default_lang', 'aZ09');
 
-				if (!empty($conf->multicompany->enabled)) {
-					if (GETPOST("superadmin")) {
-						$object->entity = 0;
-					} elseif (!empty($conf->global->MULTICOMPANY_TRANSVERSE_MODE)) {
-						$object->entity = 1; // all users in master entity
+				// Do we update also ->entity ?
+				if (!empty($conf->multicompany->enabled)) {	// If multicompany is not enabled, we never update the entity of a user.
+					if (!empty($conf->global->MULTICOMPANY_TRANSVERSE_MODE)) {
+						$object->entity = 1; // all users are in master entity
 					} else {
-						$object->entity = (!GETPOST('entity', 'int') ? 0 : GETPOST('entity', 'int'));
+						// A user should not be able to move a user into another entity. Only superadmin should be able to do this.
+						if ($user->entity == 0 && $user->admin) {
+							if (GETPOST("superadmin")) {
+								// We try to set the user as superadmin.
+								$object->entity = 0;
+							} else {
+								// We try to change the entity of user
+								$object->entity = (GETPOSTISSET('entity') ? GETPOSTINT('entity') : $object->entity);
+							}
+						}
 					}
-				} else {
-					$object->entity = (!GETPOST('entity', 'int') ? 0 : GETPOST('entity', 'int'));
 				}
 
 				// Fill array 'array_options' with data from add form
diff --git a/htdocs/user/class/user.class.php b/htdocs/user/class/user.class.php
index ec09aafeef5..1a2098619ae 100644
--- a/htdocs/user/class/user.class.php
+++ b/htdocs/user/class/user.class.php
@@ -1662,6 +1662,32 @@ class User extends CommonObject
 
 		$this->db->begin();
 
+		// Check if login already exists in same entity or into entity 0.
+		if ($this->oldcopy->login != $this->login) {
+			$sqltochecklogin = "SELECT COUNT(*) as nb FROM ".MAIN_DB_PREFIX."user WHERE entity IN (".((int) $this->entity).", 0) AND login = '".$this->db->escape($this->login)."'";
+			$resqltochecklogin = $this->db->query($sqltochecklogin);
+			if ($resqltochecklogin) {
+				$objtochecklogin = $this->db->fetch_object($resqltochecklogin);
+				if ($objtochecklogin && $objtochecklogin->nb > 0) {
+					$langs->load("errors");
+					$this->error = $langs->trans("ErrorLoginAlreadyExists", $this->login);
+					return -1;
+				}
+			}
+		}
+		if ($this->email !== '' && $this->oldcopy->email != $this->email) {
+			$sqltochecklogin = "SELECT COUNT(*) as nb FROM ".MAIN_DB_PREFIX."user WHERE entity IN (".((int) $this->entity).", 0) AND email = '".$this->db->escape($this->email)."'";
+			$resqltochecklogin = $this->db->query($sqltochecklogin);
+			if ($resqltochecklogin) {
+				$objtochecklogin = $this->db->fetch_object($resqltochecklogin);
+				if ($objtochecklogin && $objtochecklogin->nb > 0) {
+					$langs->load("errors");
+					$this->error = $langs->trans("ErrorEmailAlreadyExists", $this->email);
+					return -1;
+				}
+			}
+		}
+
 		// Update datas
 		$sql = "UPDATE ".MAIN_DB_PREFIX."user SET";
 		$sql .= " civility = '".$this->db->escape($this->civility_code)."'";
@@ -1715,7 +1741,7 @@ class User extends CommonObject
 			$sql .= ", salaryextra= ".($this->salaryextra != '' ? "'".$this->db->escape($this->salaryextra)."'" : "null");
 		}
 		$sql .= ", weeklyhours= ".($this->weeklyhours != '' ? "'".$this->db->escape($this->weeklyhours)."'" : "null");
-		$sql .= ", entity = '".$this->db->escape($this->entity)."'";
+		$sql .= ", entity = ".((int) $this->entity);
 		$sql .= ", default_range = ".($this->default_range > 0 ? $this->default_range : 'null');
 		$sql .= ", default_c_exp_tax_cat = ".($this->default_c_exp_tax_cat > 0 ? $this->default_c_exp_tax_cat : 'null');
 		$sql .= ", fk_warehouse = ".($this->fk_warehouse > 0 ? $this->fk_warehouse : "null");