CVE-2022-1345: Unrestricted Upload of File with Dangerous Type (CWE-434)
value: Stored XSS viva .svg file upload in GitHub repository causefx/organizr prior to 2.1.1810. This allows attackers to execute malicious scripts in the user's browser and it can lead to session hijacking, sensitive data exposure, and worse.
CWE description: The product allows the upload or transfer of dangerous file types that are automatically processed within its environment.
CVSS v3 base score: 9
Published: 2022-04-13T19:15Z
Repository: https://github.com/causefx/organizr
Language: PHP
Files changed: api/classes/organizr.class.php, api/functions/log-functions.php, api/functions/organizr-functions.php, api/plugins/bookmark/plugin.php
Commit date: 2022-04-11 09:52:22 -0700
Commit message: added sanitizeUserString and sanitizeEmail functions
added sanitize to uploaded image names
added sanitize to tabs, categories, users and bookmarks
removed svg files from approved image lists

Diff: diff --git a/api/classes/organizr.class.php b/api/classes/organizr.class.php
index 1cf67003..4b85009f 100644
--- a/api/classes/organizr.class.php
+++ b/api/classes/organizr.class.php
@@ -1893,11 +1893,11 @@ class Organizr
 			ini_set('upload_max_filesize', '10M');
 			ini_set('post_max_size', '10M');
 			$tempFile = $_FILES['file']['tmp_name'];
 			$targetPath = $this->root . DIRECTORY_SEPARATOR . 'data' . DIRECTORY_SEPARATOR . 'userTabs' . DIRECTORY_SEPARATOR;
 			$this->makeDir($targetPath);
-			$targetFile = $targetPath . $_FILES['file']['name'];
+			$targetFile = $targetPath . $this->sanitizeUserString($_FILES['file']['name']);
 			$this->setAPIResponse(null, pathinfo($_FILES['file']['name'], PATHINFO_BASENAME) . ' has been uploaded', null);
 			return move_uploaded_file($tempFile, $targetFile);
 		}
 	}
 
@@ -4871,11 +4871,11 @@ class Organizr
 		$array['enabled'] = ($array['enabled']) ?? 0;
 		$array['default'] = ($array['default']) ?? 0;
 		$array['type'] = ($array['type']) ?? 1;
 		$array['order'] = ($array['order']) ?? $this->getNextTabOrder() + 1;
 		if (array_key_exists('name', $array)) {
-			$array['name'] = htmlspecialchars($array['name']);
+			$array['name'] = $this->sanitizeUserString($array['name']);
 			if ($this->isTabNameTaken($array['name'])) {
 				$this->setAPIResponse('error', 'Tab name: ' . $array['name'] . ' is already taken', 409);
 				return false;
 			}
 		} else {
@@ -4921,11 +4921,11 @@ class Organizr
 		} else {
 			$this->setAPIResponse('error', 'No tab info found', 404);
 			return false;
 		}
 		if (array_key_exists('name', $array)) {
-			$array['name'] = htmlspecialchars($array['name']);
+			$array['name'] = $this->sanitizeUserString($array['name']);
 			if ($this->isTabNameTaken($array['name'], $id)) {
 				$this->setAPIResponse('error', 'Tab name: ' . $array['name'] . ' is already taken', 409);
 				return false;
 			}
 		}
@@ -4993,20 +4993,24 @@ class Organizr
 		$array = $this->checkKeys($this->getTableColumnsFormatted('categories'), $array);
 		$array['default'] = ($array['default']) ?? 0;
 		$array['order'] = ($array['order']) ?? $this->getNextCategoryOrder() + 1;
 		$array['category_id'] = ($array['category_id']) ?? $this->getNextCategoryId() + 1;
 		if (array_key_exists('category', $array)) {
+			$array['category'] = $this->sanitizeUserString($array['category']);
 			if ($this->isCategoryNameTaken($array['category'])) {
 				$this->setAPIResponse('error', 'Category name: ' . $array['category'] . ' is already taken', 409);
 				return false;
 			}
 		} else {
 			$this->setAPIResponse('error', 'Category name was not supplied', 422);
 			return false;
 		}
 		if (!array_key_exists('image', $array)) {
 			$this->setAPIResponse('error', 'Category image was not supplied', 422);
+			return false;
+		} else {
+			$array['image'] = $this->sanitizeUserString($array['image']);
 		}
 		$response = [
 			array(
 				'function' => 'query',
 				'query' => array(
@@ -5037,15 +5041,19 @@ class Organizr
 		} else {
 			$this->setAPIResponse('error', 'No category info found', 404);
 			return false;
 		}
 		if (array_key_exists('category', $array)) {
+			$array['category'] = $this->sanitizeUserString($array['category']);
 			if ($this->isCategoryNameTaken($array['category'], $id)) {
 				$this->setAPIResponse('error', 'Category name: ' . $array['category'] . ' is already taken', 409);
 				return false;
 			}
 		}
+		if (array_key_exists('image', $array)) {
+			$array['image'] = $this->sanitizeUserString($array['image']);
+		}
 		if (array_key_exists('default', $array)) {
 			if ($array['default']) {
 				$this->clearCategoryDefault();
 			}
 		}
@@ -6182,10 +6190,25 @@ class Organizr
 			$this->logger->error($e);
 		}
 		return false;
 	}
 
+	public function validateEmail($email)
+	{
+		return filter_var(trim($email), FILTER_VALIDATE_EMAIL);
+	}
+
+	public function sanitizeEmail($email)
+	{
+		return filter_var(trim($email), FILTER_SANITIZE_EMAIL);
+	}
+
+	public function sanitizeUserString($string)
+	{
+		return htmlspecialchars(trim($string));
+	}
+
 	public function updateUser($id, $array)
 	{
 		if (!$id) {
 			$this->setAPIResponse('error', 'Id was not supplied', 422);
 			return false;
@@ -6209,20 +6232,27 @@ class Organizr
 		if (array_key_exists('username', $array)) {
 			if ($array['username'] == '') {
 				$this->setAPIResponse('error', 'Username was set but empty', 409);
 				return false;
 			}
+			$array['username'] = $this->sanitizeUserString($array['username']);
 			if ($this->usernameTaken($array['username'], $array['username'], $id)) {
 				$this->setAPIResponse('error', 'Username: ' . $array['username'] . ' is already taken', 409);
 				return false;
 			}
 		}
 		if (array_key_exists('email', $array)) {
 			if ($array['email'] == '') {
 				$this->setAPIResponse('error', 'Email was set but empty', 409);
 				return false;
 			}
+			if ($this->validateEmail($array['email'])) {
+				$array['email'] = $this->sanitizeEmail($array['email']);
+			} else {
+				$this->setResponse(409, 'Email is not a valid email', ['email' => $array['email']]);
+				return false;
+			}
 			if ($this->usernameTaken($array['email'], $array['email'], $id)) {
 				$this->setAPIResponse('error', 'Email: ' . $array['email'] . ' is already taken', 409);
 				return false;
 			}
 		}
@@ -6258,10 +6288,13 @@ class Organizr
 				$this->setAPIResponse('error', 'Password was set but empty', 409);
 				return false;
 			}
 			$array['password'] = password_hash($array['password'], PASSWORD_BCRYPT);
 		}
+		if (array_key_exists('image', $array)) {
+			$array['image'] = $this->sanitizeUserString($array['image']);
+		}
 		if (array_key_exists('register_date', $array)) {
 			$this->setAPIResponse('error', 'Cannot update register date', 409);
 			return false;
 		}
 		$response = [
@@ -6315,14 +6348,34 @@ class Organizr
 		$email = $array['email'] ?? null;
 		if (!$username) {
 			$this->setAPIResponse('error', 'Username was not supplied', 409);
 			return false;
 		}
+		if ($username == '') {
+			$this->setResponse(409, 'Username was set but empty');
+			return false;
+		} else {
+			$username = $this->sanitizeUserString($username);
+		}
 		if (!$password) {
 			$this->setAPIResponse('error', 'Password was not supplied', 409);
 			return false;
 		}
+		if (!$email) {
+			$this->setAPIResponse('error', 'Email was set not supplied', 409);
+			return false;
+		}
+		if ($email == '') {
+			$this->setAPIResponse('error', 'Email was set but empty', 409);
+			return false;
+		}
+		if ($this->validateEmail($email)) {
+			$email = $this->sanitizeEmail($email);
+		} else {
+			$this->setResponse(409, 'Email is not a valid email', ['email' => $email]);
+			return false;
+		}
 		$this->setLoggerChannel('User Management');
 		if ($this->createUser($username, $password, $email)) {
 			$this->logger->info('Account created for [' . $username . ']');
 			return true;
 		} else {
@@ -6338,14 +6391,25 @@ class Organizr
 		$email = ($email) ? $email : $this->random_ascii_string(10) . '@placeholder.eml';
 		if (!$username) {
 			$this->setAPIResponse('error', 'Username was set but empty', 409);
 			return false;
 		}
+		$username = $this->sanitizeUserString($username);
 		if (!$password) {
 			$this->setAPIResponse('error', 'Password was set but empty', 409);
 			return false;
 		}
+		if ($email == '') {
+			$this->setAPIResponse('error', 'Email was set but empty', 409);
+			return false;
+		}
+		if ($this->validateEmail($email)) {
+			$email = $this->sanitizeEmail($email);
+		} else {
+			$this->setResponse(409, 'Email is not a valid email', ['email' => $email]);
+			return false;
+		}
 		if ($this->usernameTaken($username, $email)) {
 			$this->setAPIResponse('error', 'Username: ' . $username . ' or Email: ' . $email . ' is already taken', 409);
 			return false;
 		}
 		$defaults = $this->getDefaultGroup();
@@ -6395,10 +6459,11 @@ class Organizr
 		if (array_key_exists('group', $array)) {
 			if ($array['group'] == '') {
 				$this->setAPIResponse('error', 'Group was set but empty', 409);
 				return false;
 			}
+			$array['group'] = $this->sanitizeUserString($array['group']);
 			if ($this->isGroupNameTaken($array['group'], $id)) {
 				$this->setAPIResponse('error', 'Group name: ' . $array['group'] . ' is already taken', 409);
 				return false;
 			}
 		}
@@ -6474,10 +6539,11 @@ class Organizr
 		}
 		$array = $this->checkKeys($this->getTableColumnsFormatted('groups'), $array);
 		$array['default'] = ($array['default']) ?? 0;
 		$array['group_id'] = $this->getNextGroupOrder() + 1;
 		if (array_key_exists('group', $array)) {
+			$array['group'] = $this->sanitizeUserString($array['group']);
 			if ($this->isGroupNameTaken($array['group'])) {
 				$this->setAPIResponse('error', 'Group name: ' . $array['group'] . ' is already taken', 409);
 				return false;
 			}
 		} else {
diff --git a/api/functions/log-functions.php b/api/functions/log-functions.php
index b12974bc..0ef91e07 100644
--- a/api/functions/log-functions.php
+++ b/api/functions/log-functions.php
@@ -210,11 +210,11 @@ trait LogFunctions
 	public function setLoggerChannel($channel = 'Organizr', $username = null)
 	{
 		if ($this->hasDB()) {
 			$setLogger = false;
 			if ($username) {
-				$username = htmlspecialchars($username);
+				$username = $this->sanitizeUserString($username);
 			}
 			if ($this->logger) {
 				if ($channel) {
 					if (strtolower($this->logger->getChannel()) !== strtolower($channel)) {
 						$setLogger = true;
diff --git a/api/functions/organizr-functions.php b/api/functions/organizr-functions.php
index ea8a4745..7cd24858 100644
--- a/api/functions/organizr-functions.php
+++ b/api/functions/organizr-functions.php
@@ -214,11 +214,10 @@ trait OrganizrFunctions
 			switch ($ext) {
 				case 'gif':
 				case 'png':
 				case 'jpeg':
 				case 'jpg':
-				case 'svg':
 					return true;
 				default:
 					return false;
 			}
 		} elseif ($type == 'cert') {
diff --git a/api/plugins/bookmark/plugin.php b/api/plugins/bookmark/plugin.php
index 64b6b78a..5f176c3b 100644
--- a/api/plugins/bookmark/plugin.php
+++ b/api/plugins/bookmark/plugin.php
@@ -568,10 +568,11 @@ class Bookmark extends Organizr
 		$array['group_id'] = ($array['group_id']) ?? $this->getDefaultGroupId();
 		$array['category_id'] = ($array['category_id']) ?? $this->_getDefaultBookmarkCategoryId();
 		$array['enabled'] = ($array['enabled']) ?? 0;
 		$array['order'] = ($array['order']) ?? $this->_getNextBookmarkTabOrder() + 1;
 		if (array_key_exists('name', $array)) {
+			$array['name'] = $this->sanitizeUserString($array['name']);
 			if ($this->_isBookmarkTabNameTaken($array['name'])) {
 				$this->setAPIResponse('error', 'Tab name: ' . $array['name'] . ' is already taken', 409);
 				return false;
 			}
 		} else {
@@ -583,21 +584,25 @@ class Bookmark extends Organizr
 			return false;
 		}
 		if (!array_key_exists('image', $array)) {
 			$this->setAPIResponse('error', 'Tab image was not supplied', 422);
 			return false;
+		} else {
+			$array['image'] = $this->sanitizeUserString($array['image']);
 		}
 		if (array_key_exists('background_color', $array)) {
+			$array['background_color'] = $this->sanitizeUserString($array['background_color']);
 			if (!$this->_checkColorHexCode($array['background_color'])) {
 				$this->setAPIResponse('error', 'Tab background color is invalid', 422);
 				return false;
 			}
 		} else {
 			$this->setAPIResponse('error', 'Tab background color was not supplied', 422);
 			return false;
 		}
 		if (array_key_exists('text_color', $array)) {
+			$array['text_color'] = $this->sanitizeUserString($array['text_color']);
 			if (!$this->_checkColorHexCode($array['text_color'])) {
 				$this->setAPIResponse('error', 'Tab text color is invalid', 422);
 				return false;
 			}
 		} else {
@@ -634,27 +639,33 @@ class Bookmark extends Organizr
 		} else {
 			$this->setAPIResponse('error', 'No tab info found', 404);
 			return false;
 		}
 		if (array_key_exists('name', $array)) {
+			$array['name'] = $this->sanitizeUserString($array['name']);
 			if ($this->_isBookmarkTabNameTaken($array['name'], $id)) {
 				$this->setAPIResponse('error', 'Tab name: ' . $array['name'] . ' is already taken', 409);
 				return false;
 			}
 		}
 		if (array_key_exists('background_color', $array)) {
+			$array['background_color'] = $this->sanitizeUserString($array['background_color']);
 			if (!$this->_checkColorHexCode($array['background_color'])) {
 				$this->setAPIResponse('error', 'Tab background color is invalid', 422);
 				return false;
 			}
 		}
 		if (array_key_exists('text_color', $array)) {
+			$array['text_color'] = $this->sanitizeUserString($array['text_color']);
 			if (!$this->_checkColorHexCode($array['text_color'])) {
 				$this->setAPIResponse('error', 'Tab text color is invalid', 422);
 				return false;
 			}
 		}
+		if (array_key_exists('image', $array)) {
+			$array['image'] = $this->sanitizeUserString($array['image']);
+		}
 		$response = [
 			array(
 				'function' => 'query',
 				'query' => array(
 					'UPDATE `BOOKMARK-tabs` SET',
@@ -869,10 +880,11 @@ class Bookmark extends Organizr
 		$array = $this->checkKeys($this->getTableColumnsFormatted('BOOKMARK-categories'), $array);
 		$array['default'] = ($array['default']) ?? 0;
 		$array['order'] = ($array['order']) ?? $this->_getNextBookmarkCategoryOrder() + 1;
 		$array['category_id'] = ($array['category_id']) ?? $this->_getNextBookmarkCategoryId() + 1;
 		if (array_key_exists('category', $array)) {
+			$array['category'] = $this->sanitizeUserString($array['category']);
 			if ($this->_isBookmarkCategoryNameTaken($array['category'])) {
 				$this->setAPIResponse('error', 'Category name: ' . $array['category'] . ' is already taken', 409);
 				return false;
 			}
 		} else {
@@ -911,10 +923,11 @@ class Bookmark extends Organizr
 		} else {
 			$this->setAPIResponse('error', 'No category info found', 404);
 			return false;
 		}
 		if (array_key_exists('category', $array)) {
+			$array['category'] = $this->sanitizeUserString($array['category']);
 			if ($this->_isBookmarkCategoryNameTaken($array['category'], $id)) {
 				$this->setAPIResponse('error', 'Category name: ' . $array['category'] . ' is already taken', 409);
 				return false;
 			}
 		}

Vulnerable code: 			$targetFile = $targetPath . $_FILES['file']['name'];
			$array['name'] = htmlspecialchars($array['name']);
			$array['name'] = htmlspecialchars($array['name']);
				$username = htmlspecialchars($username);
				case 'svg':
Fixed code: 			$targetFile = $targetPath . $this->sanitizeUserString($_FILES['file']['name']);
			$array['name'] = $this->sanitizeUserString($array['name']);
			$array['name'] = $this->sanitizeUserString($array['name']);
			$array['category'] = $this->sanitizeUserString($array['category']);
			return false;
		} else {
			$array['image'] = $this->sanitizeUserString($array['image']);
			$array['category'] = $this->sanitizeUserString($array['category']);
		if (array_key_exists('image', $array)) {
			$array['image'] = $this->sanitizeUserString($array['image']);
		}
	public function validateEmail($email)
	{
		return filter_var(trim($email), FILTER_VALIDATE_EMAIL);
	}

	public function sanitizeEmail($email)
	{
		return filter_var(trim($email), FILTER_SANITIZE_EMAIL);
	}

	public function sanitizeUserString($string)
	{
		return htmlspecialchars(trim($string));
	}

			$array['username'] = $this->sanitizeUserString($array['username']);
			if ($this->validateEmail($array['email'])) {
				$array['email'] = $this->sanitizeEmail($array['email']);
			} else {
				$this->setResponse(409, 'Email is not a valid email', ['email' => $array['email']]);
				return false;
			}
		if (array_key_exists('image', $array)) {
			$array['image'] = $this->sanitizeUserString($array['image']);
		}
		if ($username == '') {
			$this->setResponse(409, 'Username was set but empty');
			return false;
		} else {
			$username = $this->sanitizeUserString($username);
		}
		if (!$email) {
			$this->setAPIResponse('error', 'Email was set not supplied', 409);
			return false;
		}
		if ($email == '') {
			$this->setAPIResponse('error', 'Email was set but empty', 409);
			return false;
		}
		if ($this->validateEmail($email)) {
			$email = $this->sanitizeEmail($email);
		} else {
			$this->setResponse(409, 'Email is not a valid email', ['email' => $email]);
			return false;
		}
		$username = $this->sanitizeUserString($username);
		if ($email == '') {
			$this->setAPIResponse('error', 'Email was set but empty', 409);
			return false;
		}
		if ($this->validateEmail($email)) {
			$email = $this->sanitizeEmail($email);
		} else {
			$this->setResponse(409, 'Email is not a valid email', ['email' => $email]);
			return false;
		}
			$array['group'] = $this->sanitizeUserString($array['group']);
			$array['group'] = $this->sanitizeUserString($array['group']);
				$username = $this->sanitizeUserString($username);
			$array['name'] = $this->sanitizeUserString($array['name']);
		} else {
			$array['image'] = $this->sanitizeUserString($array['image']);
			$array['background_color'] = $this->sanitizeUserString($array['background_color']);
			$array['text_color'] = $this->sanitizeUserString($array['text_color']);
			$array['name'] = $this->sanitizeUserString($array['name']);
			$array['background_color'] = $this->sanitizeUserString($array['background_color']);
			$array['text_color'] = $this->sanitizeUserString($array['text_color']);
		if (array_key_exists('image', $array)) {
			$array['image'] = $this->sanitizeUserString($array['image']);
		}
			$array['category'] = $this->sanitizeUserString($array['category']);
			$array['category'] = $this->sanitizeUserString($array['category']);