CVE-2020-10235: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: An issue was discovered in Froxlor before 0.10.14. Remote attackers with access to the installation routine could have executed arbitrary code via the database configuration options that were passed unescaped to exec, because of _backupExistingDatabase in install/lib/class.FroxlorInstall.php.
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 8.8
Published: 2020-03-09T16:15Z
Repository: https://github.com/Froxlor/Froxlor
Language: PHP
Files changed: install/lib/class.FroxlorInstall.php
Commit date: 2020-03-06 20:44:17 +0100
Commit message: forgot one escapeshellarg() and enhanced security on userdata.inc.php creation when installing

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

Diff: diff --git a/install/lib/class.FroxlorInstall.php b/install/lib/class.FroxlorInstall.php
index dc67120f5..4884a87e2 100644
--- a/install/lib/class.FroxlorInstall.php
+++ b/install/lib/class.FroxlorInstall.php
@@ -331,22 +331,22 @@ class FroxlorInstall
 		$userdata .= "\$sql['debug'] = false;\n";
 		$userdata .= "?>";
 
 		// test if we can store the userdata.inc.php in ../lib
 		$userdata_file = dirname(dirname(dirname(__FILE__))) . '/lib/userdata.inc.php';
-		if ($fp = @fopen($userdata_file, 'w')) {
-			$result = @fputs($fp, $userdata, strlen($userdata));
+		if (@touch($userdata_file) && @chmod($userdata_file, 0400) && @is_writable($userdata_file)) {
+			$fp = @fopen($userdata_file, 'w');
+			@fputs($fp, $userdata, strlen($userdata));
 			@fclose($fp);
 			$content .= $this->_status_message('green', 'OK');
-			chmod($userdata_file, 0440);
 		} else {
 			// try creating it in a temporary file
-			$temp_file = tempnam(sys_get_temp_dir(), 'fx');
-			if (touch($temp_file)) {
+			$temp_file = @tempnam(sys_get_temp_dir(), 'fx');
+			if ($temp_file) {
 				chmod($temp_file, 0400);
 				$fp = @fopen($temp_file, 'w');
-				$result = @fputs($fp, $userdata, strlen($userdata));
+				@fputs($fp, $userdata, strlen($userdata));
 				@fclose($fp);
 				$content .= $this->_status_message('orange', sprintf($this->_lng['install']['creating_configfile_temp'], $temp_file));
 			} else {
 				$content .= $this->_status_message('red', $this->_lng['install']['creating_configfile_failed']);
 				$escpduserdata = nl2br(htmlspecialchars($userdata));
@@ -566,11 +566,11 @@ class FroxlorInstall
 			$sql_query = $this->_remove_remarks($sql_query);
 			$sql_query = $this->_split_sql_file($sql_query, ';');
 			for ($i = 0; $i < sizeof($sql_query); $i ++) {
 				if (trim($sql_query[$i]) != '') {
 					try {
-						$result = $db->query($sql_query[$i]);
+						$db->query($sql_query[$i]);
 					} catch (\PDOException $e) {
 						$content .= $this->_status_message('red', $e->getMessage());
 						$fatal_fail = true;
 						$this->_abort = true;
 						break;
@@ -733,11 +733,11 @@ class FroxlorInstall
 				$do_backup = true;
 				$mysql_dump = '/usr/local/bin/mysqldump';
 			}
 
 			if ($do_backup) {
-				$command = $mysql_dump . " " . escapeshellarg($this->_data['mysql_database']) . " -u " . escapeshellarg($this->_data['mysql_root_user']) . " --password='" . $this->_data['mysql_root_pass'] . "' --result-file=" . $filename;
+				$command = $mysql_dump . " " . escapeshellarg($this->_data['mysql_database']) . " -u " . escapeshellarg($this->_data['mysql_root_user']) . " --password='" . escapeshellarg($this->_data['mysql_root_pass']) . "' --result-file=" . $filename;
 				$output = exec($command);
 				if (stristr($output, "error")) {
 					$content .= $this->_status_message('red', $this->_lng['install']['backup_failed']);
 				} else {
 					$content .= $this->_status_message('green', 'OK (' . $filename . ')');

Vulnerable code: 		if ($fp = @fopen($userdata_file, 'w')) {
			$result = @fputs($fp, $userdata, strlen($userdata));
			chmod($userdata_file, 0440);
			$temp_file = tempnam(sys_get_temp_dir(), 'fx');
			if (touch($temp_file)) {
				$result = @fputs($fp, $userdata, strlen($userdata));
						$result = $db->query($sql_query[$i]);
				$command = $mysql_dump . " " . escapeshellarg($this->_data['mysql_database']) . " -u " . escapeshellarg($this->_data['mysql_root_user']) . " --password='" . $this->_data['mysql_root_pass'] . "' --result-file=" . $filename;
Fixed code: 		if (@touch($userdata_file) && @chmod($userdata_file, 0400) && @is_writable($userdata_file)) {
			$fp = @fopen($userdata_file, 'w');
			@fputs($fp, $userdata, strlen($userdata));
			$temp_file = @tempnam(sys_get_temp_dir(), 'fx');
			if ($temp_file) {
				@fputs($fp, $userdata, strlen($userdata));
						$db->query($sql_query[$i]);
				$command = $mysql_dump . " " . escapeshellarg($this->_data['mysql_database']) . " -u " . escapeshellarg($this->_data['mysql_root_user']) . " --password='" . escapeshellarg($this->_data['mysql_root_pass']) . "' --result-file=" . $filename;