CVE-2020-10235: Improper Encoding or Escaping of Output (CWE-116)
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 prepares a structured message for communication with another component, but encoding or escaping of the data is either missing or done incorrectly. As a result, the intended structure of the message is not preserved.
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;