CVE-2023-0877: Improper Control of Generation of Code ('Code Injection') (CWE-94)
value: Code Injection in GitHub repository froxlor/froxlor prior to 2.0.11.
CWE description: The product constructs all or part of a code segment using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the syntax or behavior of the intended code segment.
CVSS v3 base score: 8.8
Published: 2023-02-17T01:15Z
Repository: https://github.com/froxlor/froxlor
Language: PHP
Files changed: lib/Froxlor/Froxlor.php, lib/Froxlor/SImExporter.php, lib/Froxlor/Settings.php, lib/Froxlor/UI/Form.php
Commit date: 2023-02-07 13:02:11 +0100
Commit message: run Form::processForm() when importing settings so the same validations apply if the import file has malicious content

Signed-off-by: Michael Kaufmann <d00p@froxlor.org>

Diff: diff --git a/lib/Froxlor/Froxlor.php b/lib/Froxlor/Froxlor.php
index 18d3b8ea22..1322e80045 100644
--- a/lib/Froxlor/Froxlor.php
+++ b/lib/Froxlor/Froxlor.php
@@ -87,11 +87,11 @@ final class Froxlor
 	 *
 	 * @param string $to_check version to check, if empty current version is used
 	 *
 	 * @return bool true if version to check does not match, else false
 	 */
-	public static function hasUpdates(string $to_check): bool
+	public static function hasUpdates(string $to_check = ''): bool
 	{
 		if (empty($to_check)) {
 			$to_check = self::VERSION;
 		}
 		if (Settings::Get('panel.version') == null || Settings::Get('panel.version') != $to_check) {
@@ -107,11 +107,11 @@ final class Froxlor
 	 *
 	 * @param string $to_check version to check, if empty current dbversion is used
 	 *
 	 * @return bool true if version to check does not match, else false
 	 */
-	public static function hasDbUpdates(string $to_check): bool
+	public static function hasDbUpdates(string $to_check = ''): bool
 	{
 		if (empty($to_check)) {
 			$to_check = self::DBVERSION;
 		}
 		if (Settings::Get('panel.db_version') == null || Settings::Get('panel.db_version') != $to_check) {
diff --git a/lib/Froxlor/SImExporter.php b/lib/Froxlor/SImExporter.php
index 458b8b5b80..0a7884cae3 100644
--- a/lib/Froxlor/SImExporter.php
+++ b/lib/Froxlor/SImExporter.php
@@ -25,10 +25,11 @@
 
 namespace Froxlor;
 
 use Exception;
 use Froxlor\Database\Database;
+use Froxlor\UI\Form;
 use PDO;
 
 /**
  * Class SImExporter
  *
@@ -77,18 +78,20 @@ class SImExporter
 			$index = $row['settinggroup'] . "." . $row['varname'];
 			if (!in_array($index, self::$no_export)) {
 				$_data[$index] = $row['value'];
 			}
 
-			if (array_key_exists($row['settinggroup'], $settings_definitions) && array_key_exists($row['varname'], $settings_definitions[$row['settinggroup']])) {
+			if (array_key_exists($row['settinggroup'], $settings_definitions) && array_key_exists($row['varname'],
+					$settings_definitions[$row['settinggroup']])) {
 				// Export image file
 				if ($settings_definitions[$row['settinggroup']][$row['varname']]['type'] === "image") {
 					if ($row['value'] === "") {
 						continue;
 					}
 
-					$_data[$index . '.image_data'] = base64_encode(file_get_contents(explode('?', $row['value'], 2)[0]));
+					$_data[$index . '.image_data'] = base64_encode(file_get_contents(explode('?', $row['value'],
+						2)[0]));
 				}
 			}
 		}
 
 		// add checksum for validation
@@ -138,69 +141,38 @@ class SImExporter
 					$_data['system.leenabled'] = 0;
 					$_data['system.le_froxlor_enabled'] = 0;
 					$_data['system.le_froxlor_redirect'] = 0;
 				}
 			}
-			// store new data
-			foreach ($_data as $index => $value) {
-				$index_split = explode('.', $index, 3);
-
-				// Catch image_data and save it
-				if (isset($index_split[2]) && $index_split[2] === 'image_data' && !empty($_data[$index_split[0] . '.' . $index_split[1]])) {
-					$path = Froxlor::getInstallDir() . '/img/';
-					if (!is_dir($path) && !mkdir($path, 0775)) {
-						throw new Exception("img directory does not exist and cannot be created");
-					}
-
-					// Make sure we can write to the upload directory
-					if (!is_writable($path)) {
-						if (!chmod($path, 0775)) {
-							throw new Exception("Cannot write to img directory");
-						}
-					}
-
-					$img_data = base64_decode($value);
-					$img_filename = Froxlor::getInstallDir() . '/' . str_replace('../', '', explode('?', $_data[$index_split[0] . '.' . $index_split[1]], 2)[0]);
 
-					file_put_contents($img_filename, $img_data);
-
-					if (function_exists('finfo_open')) {
-						$finfo = finfo_open(FILEINFO_MIME_TYPE);
-						$mimetype = finfo_file($finfo, $img_filename);
-						finfo_close($finfo);
-					} else {
-						$mimetype = mime_content_type($img_filename);
-					}
-					if (empty($mimetype)) {
-						$mimetype = 'application/octet-stream';
+			$form_data = [];
+			// read in all current settings
+			$current_settings = Settings::getAll();
+			foreach ($current_settings as $setting_group => $setting) {
+				foreach ($setting as $varname => $value) {
+					// set all group/varname:values which are not in the import file
+					if (!isset($_data[$setting_group.'.'.$varname])) {
+						$_data[$setting_group.'.'.$varname] = $value;
 					}
-					if (!in_array($mimetype, ['image/jpeg', 'image/jpg', 'image/png', 'image/gif'])) {
-						@unlink($img_filename);
-						throw new Exception("Uploaded file is not a valid image");
-					}
-
-					$spl = explode('.', $img_filename);
-					$file_extension = strtolower(array_pop($spl));
-					unset($spl);
-
-					if (!in_array($file_extension, [
-						'jpeg',
-						'jpg',
-						'png',
-						'gif'
-					])) {
-						@unlink($img_filename);
-						throw new Exception("Invalid file-extension, use one of: jpeg, jpg, png, gif");
-					}
-					continue;
 				}
+			}
+			// re-format the array-key for Form::processForm
+			foreach ($_data as $key => $value) {
+				$form_data[str_replace(".", "_", $key)] = $value;
+			}
 
-				Settings::Set($index, $value);
+			// store new data
+			$settings_data = PhpHelper::loadConfigArrayDir(Froxlor::getInstallDir() . '/actions/admin/settings/');
+			Settings::loadSettingsInto($settings_data);
+
+			if (Form::processForm($settings_data, $form_data, [], null, true)) {
+				// save to DB
+				Settings::Flush();
+				// all good
+				return true;
+			} else {
+				throw new Exception("Importing settings failed");
 			}
-			// save to DB
-			Settings::Flush();
-			// all good
-			return true;
 		}
 		throw new Exception("Invalid JSON data: " . json_last_error_msg());
 	}
 }
diff --git a/lib/Froxlor/Settings.php b/lib/Froxlor/Settings.php
index 2e2176bb84..d24a1c8a81 100644
--- a/lib/Froxlor/Settings.php
+++ b/lib/Froxlor/Settings.php
@@ -327,10 +327,16 @@ class Settings
 				}
 			}
 		}
 	}
 
+	public static function getAll() : array
+	{
+		self::init();
+		return self::$data;
+	}
+
 	/**
 	 * get value from config by identifier
 	 */
 	public static function Config(string $config)
 	{
diff --git a/lib/Froxlor/UI/Form.php b/lib/Froxlor/UI/Form.php
index 8ab65fb724..6cf40ab8cf 100644
--- a/lib/Froxlor/UI/Form.php
+++ b/lib/Froxlor/UI/Form.php
@@ -201,11 +201,11 @@ class Form
 			}
 		}
 		return $returnvalue;
 	}
 
-	public static function processForm(&$form, &$input, $url_params = [], $part = null, $settings_all = [], $settings_part = null, $only_enabledisable = false)
+	public static function processForm(&$form, &$input, $url_params = [], $part = null, bool $settings_all = false, $settings_part = null, bool $only_enabledisable = false)
 	{
 		if (\Froxlor\Validate\Form::validateFormDefinition($form)) {
 			$submitted_fields = [];
 			$changed_fields = [];
 			$saved_fields = [];

Vulnerable code: 	public static function hasUpdates(string $to_check): bool
	public static function hasDbUpdates(string $to_check): bool
			if (array_key_exists($row['settinggroup'], $settings_definitions) && array_key_exists($row['varname'], $settings_definitions[$row['settinggroup']])) {
					$_data[$index . '.image_data'] = base64_encode(file_get_contents(explode('?', $row['value'], 2)[0]));
			// store new data
			foreach ($_data as $index => $value) {
				$index_split = explode('.', $index, 3);

				// Catch image_data and save it
				if (isset($index_split[2]) && $index_split[2] === 'image_data' && !empty($_data[$index_split[0] . '.' . $index_split[1]])) {
					$path = Froxlor::getInstallDir() . '/img/';
					if (!is_dir($path) && !mkdir($path, 0775)) {
						throw new Exception("img directory does not exist and cannot be created");
					}

					// Make sure we can write to the upload directory
					if (!is_writable($path)) {
						if (!chmod($path, 0775)) {
							throw new Exception("Cannot write to img directory");
						}
					}

					$img_data = base64_decode($value);
					$img_filename = Froxlor::getInstallDir() . '/' . str_replace('../', '', explode('?', $_data[$index_split[0] . '.' . $index_split[1]], 2)[0]);
					file_put_contents($img_filename, $img_data);

					if (function_exists('finfo_open')) {
						$finfo = finfo_open(FILEINFO_MIME_TYPE);
						$mimetype = finfo_file($finfo, $img_filename);
						finfo_close($finfo);
					} else {
						$mimetype = mime_content_type($img_filename);
					}
					if (empty($mimetype)) {
						$mimetype = 'application/octet-stream';
					if (!in_array($mimetype, ['image/jpeg', 'image/jpg', 'image/png', 'image/gif'])) {
						@unlink($img_filename);
						throw new Exception("Uploaded file is not a valid image");
					}

					$spl = explode('.', $img_filename);
					$file_extension = strtolower(array_pop($spl));
					unset($spl);

					if (!in_array($file_extension, [
						'jpeg',
						'jpg',
						'png',
						'gif'
					])) {
						@unlink($img_filename);
						throw new Exception("Invalid file-extension, use one of: jpeg, jpg, png, gif");
					}
					continue;
				Settings::Set($index, $value);
			// save to DB
			Settings::Flush();
			// all good
			return true;
	public static function processForm(&$form, &$input, $url_params = [], $part = null, $settings_all = [], $settings_part = null, $only_enabledisable = false)
Fixed code: 	public static function hasUpdates(string $to_check = ''): bool
	public static function hasDbUpdates(string $to_check = ''): bool
use Froxlor\UI\Form;
			if (array_key_exists($row['settinggroup'], $settings_definitions) && array_key_exists($row['varname'],
					$settings_definitions[$row['settinggroup']])) {
					$_data[$index . '.image_data'] = base64_encode(file_get_contents(explode('?', $row['value'],
						2)[0]));
			$form_data = [];
			// read in all current settings
			$current_settings = Settings::getAll();
			foreach ($current_settings as $setting_group => $setting) {
				foreach ($setting as $varname => $value) {
					// set all group/varname:values which are not in the import file
					if (!isset($_data[$setting_group.'.'.$varname])) {
						$_data[$setting_group.'.'.$varname] = $value;
			}
			// re-format the array-key for Form::processForm
			foreach ($_data as $key => $value) {
				$form_data[str_replace(".", "_", $key)] = $value;
			}
			// store new data
			$settings_data = PhpHelper::loadConfigArrayDir(Froxlor::getInstallDir() . '/actions/admin/settings/');
			Settings::loadSettingsInto($settings_data);

			if (Form::processForm($settings_data, $form_data, [], null, true)) {
				// save to DB
				Settings::Flush();
				// all good
				return true;
			} else {
				throw new Exception("Importing settings failed");
	public static function getAll() : array
	{
		self::init();
		return self::$data;
	}

	public static function processForm(&$form, &$input, $url_params = [], $part = null, bool $settings_all = false, $settings_part = null, bool $only_enabledisable = false)