Froxlor has privilege escalation in SSH key synchronization via symlinked `authorized_keys` path

@@ -312,7 +312,7 @@ public static function getUnknownDomainTemplate(string $servername = "")
 	public static function storeDefaultIndex(
 		string $loginname,
 		string $destination,
-			   $logger = null,
+		       $logger = null,
 		bool   $force = false
 	)
 	{
@@ -391,6 +391,8 @@ public static function makeCorrectFile(string $filename, string $fixed_homedir =
 			$filename = '/' . $filename;
 		}
 
+		$filename = FileDir::makeCorrectDir(dirname($filename)) . '/' . basename($filename);
+
 		// if given, check that the target file is within the $fixed_homedir
 		// by checking each folder and the file for being a symlink and whether it targets
 		// the customers homedir or points outside of it
@@ -420,7 +422,8 @@ public static function makeCorrectFile(string $filename, string $fixed_homedir =
 			}
 			// check whether file is symlink itself
 			if (is_link($filename)) {
-				$check_dir = readlink($check_dir);
+				$filename = readlink($filename);
+				$check_dir = FileDir::makeCorrectDir(dirname($filename), $fixed_homedir);
 				if (substr($check_dir, 0, strlen($fixed_homedir)) != $fixed_homedir) {
 					throw new Exception("Found symlink pointing outside of customer home directory: " . substr($filename, strlen($fixed_homedir)));
 				}

@@ -82,12 +82,12 @@
 		$return['system_webserver_serveradmin'] = [
 			'type' => 'select',
 			'select_var' => [
-				'customer' => lng('admin.webserver_serveradmin.customer'),
-				'admin' => lng('admin.webserver_serveradmin.admin'),
-				'global' => lng('admin.webserver_serveradmin.global'),
-				'none' => lng('admin.webserver_serveradmin.none')
+				'customer' => 'Customer email address (default)',
+				'admin' => 'Admin email address',
+				'global' => 'Panel admin email address',
+				'none' => 'No ServerAdmin'
 			],
-			'selected' => 'apache2',
+			'selected' => 'customer',
 			'label' => $question,
 			'prior_infotext' => $description
 		];

@@ -145,8 +145,6 @@ public function add()
 			}
 		}
 
-		// TODO regex validate content for invalid characters
-
 		if ($ttl <= 0) {
 			$ttl = 18000;
 		}
@@ -156,6 +154,11 @@ public function add()
 			$errors[] = lng('error.dns_content_empty');
 		}
 
+		// remove invalid control characters (allow tab + printable ASCII)
+		$content = preg_replace('/[^\x09\x20-\x7E]/', '', $content);
+		// collapse excessive whitespace
+		$content = preg_replace('/\s+/', ' ', $content);
+
 		if ($type != 'CNAME') {
 			// check whether there is a CNAME-record for the same resource
 			foreach ($dom_entries as $existing_entries) {

@@ -118,6 +118,10 @@ public function add()
 
 			if (Settings::Get('system.allow_customer_shell') == '1' && $customer['shell_allowed'] == '1') {
 				$shell = Validate::validate(trim($shell), 'shell', '', '', [], true);
+				$availableshells = explode(',', Settings::Get('system.available_shells'));
+				if (!is_array($availableshells) || empty($availableshells) || !in_array($shell, $availableshells)) {
+					$shell = "/bin/false";
+				}
 			} else {
 				$shell = "/bin/false";
 			}
@@ -438,6 +442,10 @@ public function update()
 
 		if (Settings::Get('system.allow_customer_shell') == '1' && $customer['shell_allowed'] == '1') {
 			$shell = Validate::validate(trim($shell), 'shell', '', '', [], true);
+			$availableshells = explode(',', Settings::Get('system.available_shells'));
+			if (!is_array($availableshells) || empty($availableshells) || !in_array($shell, $availableshells)) {
+				$shell = "/bin/false";
+			}
 		} else {
 			$shell = "/bin/false";
 		}

@@ -33,6 +33,9 @@
 
 class SshKeys
 {
+	/**
+	 * @throws \Exception
+	 */
 	public static function generateFiles(&$cronlog)
 	{
 		if (intval(Settings::Get('system.allow_customer_shell')) == 0) {
@@ -50,7 +53,8 @@ public static function generateFiles(&$cronlog)
 			SELECT `id`, `ssh_pubkey` FROM `" . TABLE_PANEL_USER_SSHKEYS . "` WHERE `ftp_user_id` = :fuid AND `customerid` = :cid
 		");
 		while ($usr = $sel_stmt->fetch(PDO::FETCH_ASSOC)) {
-			$authkeysfile = FileDir::makeCorrectFile($usr['homedir'] . '/.ssh/authorized_keys');
+			$userHomeDir = FileDir::makeCorrectDir($usr['homedir'] . '/.ssh', $usr['homedir']);
+			$authkeysfile = FileDir::makeCorrectFile($userHomeDir . '/authorized_keys', $usr['homedir']);
 			$cronlog->logAction(FroxlorLogger::CRON_ACTION, LOG_NOTICE, 'Creating file ' . $authkeysfile);
 			// remove all entries with 'froxlor:id=...'
 			self::removeFroxlorKeys($authkeysfile, $cronlog);

@@ -370,10 +370,12 @@ public static function storeDefaultIndex(
 	 * Function which returns a correct filename, means to add a slash at the beginning if there wasn't one
 	 *
 	 * @param string $filename the filename
+	 * @param string $fixed_homedir whether to check that the given file is within the fixed home-directory
 	 *
 	 * @return string the corrected filename
+	 * @throws Exception
 	 */
-	public static function makeCorrectFile(string $filename): string
+	public static function makeCorrectFile(string $filename, string $fixed_homedir = ""): string
 	{
 		if (trim($filename) == '') {
 			$error = 'Given filename for function ' . __FUNCTION__ . ' is empty.' . "\n";
@@ -389,6 +391,42 @@ public static function makeCorrectFile(string $filename): string
 			$filename = '/' . $filename;
 		}
 
+		// if given, check that the target file is within the $fixed_homedir
+		// by checking each folder and the file for being a symlink and whether it targets
+		// the customers homedir or points outside of it
+		if (!empty($fixed_homedir)) {
+			$to_check = explode("/", substr($filename, strlen(self::makeCorrectDir($fixed_homedir))), -1);
+			$check_dir = substr($fixed_homedir, -1) == '/' ? substr($fixed_homedir, 0, -1) : $fixed_homedir;
+			// Symlink check
+			foreach ($to_check as $sub_dir) {
+				$check_dir .= '/' . $sub_dir;
+				if (is_link($check_dir)) {
+					$original_target = $check_dir;
+					$check_dir = readlink($check_dir);
+					$link_dir = dirname($original_target);
+					// check whether the link is relative or absolute
+					if (substr($check_dir, 0, 1) != '/') {
+						// relative directory, prepend link_dir
+						$check_dir = $link_dir . '/' . $check_dir;
+					}
+					if (substr($check_dir, 0, strlen($fixed_homedir)) != $fixed_homedir) {
+						throw new Exception("Found symlink pointing outside of customer home directory: " . substr($original_target, strlen($fixed_homedir)));
+					}
+				}
+			}
+			// check for the path to be within the given homedir
+			if (substr($filename, 0, strlen($fixed_homedir)) != $fixed_homedir) {
+				throw new Exception("Target path/file not within the required customer home directory");
+			}
+			// check whether file is symlink itself
+			if (is_link($filename)) {
+				$check_dir = readlink($check_dir);
+				if (substr($check_dir, 0, strlen($fixed_homedir)) != $fixed_homedir) {
+					throw new Exception("Found symlink pointing outside of customer home directory: " . substr($filename, strlen($fixed_homedir)));
+				}
+			}
+		}
+
 		return self::makeSecurePath($filename);
 	}
 

@@ -390,18 +390,18 @@ public static function validateBase64Image(string $base64string)
 	public static function validateDnsLoc(string $input)
 	{
 		$pattern = '/^
-        (\d{1,2})\s+                # latitude degrees
-        (\d{1,2})\s+                # latitude minutes
-        (\d{1,2}(?:\.\d+)?)\s+      # latitude seconds
-        ([NS])\s+                   # latitude direction
-        (\d{1,3})\s+                # longitude degrees
-        (\d{1,2})\s+                # longitude minutes
-        (\d{1,2}(?:\.\d+)?)\s+      # longitude seconds
-        ([EW])\s+                   # longitude direction
+        (\d{1,2})[ \t]+                # latitude degrees
+        (\d{1,2})[ \t]+                # latitude minutes
+        (\d{1,2}(?:\.\d+)?)[ \t]+      # latitude seconds
+        ([NS])[ \t]+                   # latitude direction
+        (\d{1,3})[ \t]+                # longitude degrees
+        (\d{1,2})[ \t]+                # longitude minutes
+        (\d{1,2}(?:\.\d+)?)[ \t]+      # longitude seconds
+        ([EW])[ \t]+                   # longitude direction
         (-?\d+(?:\.\d+)?)m          # altitude
-        (?:\s+(\d+(?:\.\d+)?)m      # size (optional)
-        (?:\s+(\d+(?:\.\d+)?)m      # horiz precision (optional)
-        (?:\s+(\d+(?:\.\d+)?)m)?    # vert precision (optional)
+        (?:[ \t]+(\d+(?:\.\d+)?)m      # size (optional)
+        (?:[ \t]+(\d+(?:\.\d+)?)m      # horiz precision (optional)
+        (?:[ \t]+(\d+(?:\.\d+)?)m)?    # vert precision (optional)
         )?)?$/x';
 
 		if (!preg_match($pattern, $input, $matches)) {
@@ -551,7 +551,7 @@ public static function validateDnsTlsa(string $input)
 			return false; // SHA-512
 		}
 
-		if ($matchingType === 0 && strlen($data) < 2) {
+		if ($matchingType === 0 && (strlen($data) < 2 || strlen($data) > 4096)) {
 			return false; // at least 1 byte hex
 		}