CVE-2024-24572: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') (CWE-89)
value: facileManager is a modular suite of web apps built with the sysadmin in mind. In versions 4.5.0 and earlier, the $_REQUEST global array was unsafely called inside an extract() function in admin-logs.php. The PHP file fm-init.php prevents arbitrary manipulation of $_SESSION via the GET/POST parameters. However, it does not prevent manipulation of any other sensitive variables such as $search_sql. Knowing this, an authenticated user with privileges to view site logs can manipulate the search_sql
variable by appending a GET parameter search_sql in the URL. The information above means that the checks and SQL injection prevention attempts were rendered unusable.
CWE description: The product constructs all or part of an SQL command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended SQL command when it is sent to a downstream component. Without sufficient removal or quoting of SQL syntax in user-controllable inputs, the generated SQL query can cause those inputs to be interpreted as SQL instead of ordinary user data.
CVSS v3 base score: 6.5
Published: 2024-01-31T23:15Z
Repository: https://github.com/WillyXJ/facileManager
Language: PHP
Files changed: server/fm-includes/version.php, server/fm-modules/facileManager/change.log, server/fm-modules/facileManager/classes/class_logins.php, server/fm-modules/facileManager/classes/class_users.php, server/fm-modules/facileManager/functions.php, server/fm-modules/facileManager/pages/admin-logs.php
Commit date: 2024-01-30 13:26:16 -0700
Commit message: fM - #599 - Multiple security fixes

Diff: diff --git a/server/fm-includes/version.php b/server/fm-includes/version.php
index 5f70e762..a564c65a 100644
--- a/server/fm-includes/version.php
+++ b/server/fm-includes/version.php
@@ -22,11 +22,11 @@
 /**
  * The facileManager version string
  *
  * @global string $fm_version
  */
-$fm_version = '4.5.0';
+$fm_version = '4.5.1';
 
 /**
  * Holds the facileManager DB revision, increments when changes are made to the facileManager DB schema.
  *
  * @global int $fm_db_version
diff --git a/server/fm-modules/facileManager/change.log b/server/fm-modules/facileManager/change.log
index 58c88f45..af8da5d1 100644
--- a/server/fm-modules/facileManager/change.log
+++ b/server/fm-modules/facileManager/change.log
@@ -1,5 +1,11 @@
+4.5.1 (2024-01-30)
+==================
+* Server - [security] Fixed authenticated variable manipulation leading to SQL Injection. (Issue #599)
+* Server - [security] Fixed privilege escalation via mass assignment. (Issue #599)
+* Server - [security] Fixed systemic cross-site scripting (XSS). (Issue #599)
+
 4.5.0 (2023-04-19)
 ==================
 * Server - [improvement] The settings now displays the timestamp of the last software update check.
 * Server - [bug] Fixed module settings page when the has no settings.
 
diff --git a/server/fm-modules/facileManager/classes/class_logins.php b/server/fm-modules/facileManager/classes/class_logins.php
index 630ce727..bb8bef15 100644
--- a/server/fm-modules/facileManager/classes/class_logins.php
+++ b/server/fm-modules/facileManager/classes/class_logins.php
@@ -260,11 +260,11 @@ class fm_login {
 		$fm_db_version = getOption('fm_db_version');
 		$auth_method = ($fm_db_version >= 18) ? getOption('auth_method') : true;
 		if ($auth_method) {
 			/** Use Builtin Auth when Default Auth Method is LDAP but user is defined with 'facileManager/Builtin' */
 			$result = $fmdb->query("SELECT * FROM `fm_users` WHERE `user_login` = '$user_login' and `user_auth_type`=1 and `user_status`='active'");
-			if (is_array($fmdb->last_result) && $fmdb->last_result[0]->user_login == $user_login) {
+			if (isset($fmdb->last_result) && is_array($fmdb->last_result) && $fmdb->last_result[0]->user_login == $user_login) {
 				$auth_method = 1;
 			}
 
 			/** Builtin Authentication */
 			if ($auth_method == 1) {
diff --git a/server/fm-modules/facileManager/classes/class_users.php b/server/fm-modules/facileManager/classes/class_users.php
index 024222ac..375e12f9 100644
--- a/server/fm-modules/facileManager/classes/class_users.php
+++ b/server/fm-modules/facileManager/classes/class_users.php
@@ -251,12 +251,15 @@ class fm_users {
 		}
 
 		$post['user_id'] = (!isset($post['user_id']) || !$post['user_id']) ? $_SESSION['user']['id'] : intval($post['user_id']);
 
 		/** Authorized to update users? */
-		if ((!currentUserCan('manage_users') && $_SESSION['user']['id'] != $post['user_id']) || isset($post['user_login'])) {
-			return _('You do not have permission to make these changes.');
+		if ((!currentUserCan('manage_users') && ($_SESSION['user']['id'] != $post['user_id'] || isset($post['user_caps']))) 
+			|| ($_SESSION['user']['id'] != $post['user_id'] && $post['user_id'] == getDefaultAdminID()) 
+			|| isset($post['user_login'])
+			|| (isset($post['user_caps']) && $_SESSION['user']['id'] == $post['user_id'] && $_SESSION['user']['id'] == getDefaultAdminID())) {
+				return _('You do not have permission to make these changes.');
 		}
 
 		if (!empty($post['user_password'])) {
 			if (empty($post['cpassword']) || $post['user_password'] != $post['cpassword']) return _('Passwords do not match.');
 			$post['user_password'] = sanitize($post['user_password']);
diff --git a/server/fm-modules/facileManager/functions.php b/server/fm-modules/facileManager/functions.php
index 08e3ab4f..d3e8ebe0 100644
--- a/server/fm-modules/facileManager/functions.php
+++ b/server/fm-modules/facileManager/functions.php
@@ -214,15 +214,16 @@ function sanitize($data, $replace = null) {
 		$strip_chars = array("'", "\"", "`", "$", "?", "*", "&", "^", "!", "#");
 		$replace_chars = array(" ", "\\", "_", "(", ")", ",", ".", "-");
 
 		$data = str_replace($strip_chars, '', $data);
 		$data = str_replace($replace_chars, $replace, $data);
-		$data = str_replace('--', '-', $data);
+		$data = str_replace($replace . $replace, $replace, $data);
 		
 		return $data;
 	} else {
 		if (is_string($data)) {
+			$data = htmlspecialchars(strip_tags($data), ENT_NOQUOTES);
 			if ($fmdb->use_mysqli) {
 				return @mysqli_real_escape_string($fmdb->dbh, $data);
 			} else {
 				return @mysql_real_escape_string($data);
 			}
diff --git a/server/fm-modules/facileManager/pages/admin-logs.php b/server/fm-modules/facileManager/pages/admin-logs.php
index 80b0d36e..28e717e2 100644
--- a/server/fm-modules/facileManager/pages/admin-logs.php
+++ b/server/fm-modules/facileManager/pages/admin-logs.php
@@ -26,20 +26,19 @@ if (!currentUserCan('view_logs')) unAuth();
 printHeader();
 @printMenu();
 
 $response = isset($response) ? $response : null;
 
-$search_sql = $list = $log_search_query = $log_search_date_b = $log_search_date_e = null;
 extract($_REQUEST);
+$search_sql = null;
 
 /** Module search */
 if (isset($log_search_module) && is_array($log_search_module) && !in_array('All Modules', $log_search_module)) {
 	$search_sql .= 'AND log_module IN ("' . join('","', $log_search_module) . '") ';
 }
 /** User search */
 $default_timezone = getOption('timezone', $_SESSION['user']['account_id']);
-$list = null;
 if (isset($log_search_user) && is_array($log_search_user) && !in_array('0', $log_search_user)) {
 	$search_sql .= 'AND user_login IN ("' . join('","', $log_search_user) . '") ';
 }
 /** Begin date search */
 if (isset($log_search_date_b) && !empty($log_search_date_b)) {

Vulnerable code: $fm_version = '4.5.0';
			if (is_array($fmdb->last_result) && $fmdb->last_result[0]->user_login == $user_login) {
		if ((!currentUserCan('manage_users') && $_SESSION['user']['id'] != $post['user_id']) || isset($post['user_login'])) {
			return _('You do not have permission to make these changes.');
		$data = str_replace('--', '-', $data);
$search_sql = $list = $log_search_query = $log_search_date_b = $log_search_date_e = null;
$list = null;
Fixed code: $fm_version = '4.5.1';
4.5.1 (2024-01-30)
==================
* Server - [security] Fixed authenticated variable manipulation leading to SQL Injection. (Issue #599)
* Server - [security] Fixed privilege escalation via mass assignment. (Issue #599)
* Server - [security] Fixed systemic cross-site scripting (XSS). (Issue #599)

			if (isset($fmdb->last_result) && is_array($fmdb->last_result) && $fmdb->last_result[0]->user_login == $user_login) {
		if ((!currentUserCan('manage_users') && ($_SESSION['user']['id'] != $post['user_id'] || isset($post['user_caps']))) 
			|| ($_SESSION['user']['id'] != $post['user_id'] && $post['user_id'] == getDefaultAdminID()) 
			|| isset($post['user_login'])
			|| (isset($post['user_caps']) && $_SESSION['user']['id'] == $post['user_id'] && $_SESSION['user']['id'] == getDefaultAdminID())) {
				return _('You do not have permission to make these changes.');
		$data = str_replace($replace . $replace, $replace, $data);
			$data = htmlspecialchars(strip_tags($data), ENT_NOQUOTES);
$search_sql = null;