CVE-2023-1033: Cross-Site Request Forgery (CSRF) (CWE-352)
value: Cross-Site Request Forgery (CSRF) in GitHub repository froxlor/froxlor prior to 2.0.11.
CWE description: The web application does not, or can not, sufficiently verify whether a well-formed, valid, consistent request was intentionally provided by the user who submitted the request.
CVSS v3 base score: 8.8
Published: 2023-02-25T01:15Z
Repository: https://github.com/froxlor/froxlor
Language: PHP
Files changed: lib/Froxlor/Ajax/Ajax.php, lib/Froxlor/UI/Listing.php, lib/tablelisting/admin/tablelisting.domains.php, lib/tablelisting/customer/tablelisting.htaccess.php, lib/tablelisting/customer/tablelisting.mysqls.php, lib/tablelisting/tablelisting.dns.php
Commit date: 2023-02-14 11:36:11 +0100
Commit message: check for existing fields when setting/updating tablelisting-columns

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

Diff: diff --git a/lib/Froxlor/Ajax/Ajax.php b/lib/Froxlor/Ajax/Ajax.php
index 6e54a9e168..a5483c1971 100644
--- a/lib/Froxlor/Ajax/Ajax.php
+++ b/lib/Froxlor/Ajax/Ajax.php
@@ -235,23 +235,23 @@ class Ajax
 	}
 
 	private function updateTablelisting()
 	{
 		$columns = [];
-		foreach ((Request::any('columns') ?? []) as $value) {
+		foreach ((Request::post('columns') ?? []) as $value) {
 			$columns[] = $value;
 		}
 		if (!empty($columns)) {
-			Listing::storeColumnListingForUser([Request::any('listing') => $columns]);
+			$columns = Listing::storeColumnListingForUser([Request::post('listing') => $columns]);
 			return $this->jsonResponse($columns);
 		}
 		return $this->errorResponse('At least one column must be selected', 406);
 	}
 
 	private function resetTablelisting()
 	{
-		Listing::deleteColumnListingForUser([Request::any('listing') => []]);
+		Listing::deleteColumnListingForUser([Request::post('listing') => []]);
 		return $this->jsonResponse([]);
 	}
 
 	private function editApiKey()
 	{
diff --git a/lib/Froxlor/UI/Listing.php b/lib/Froxlor/UI/Listing.php
index 0d95ab693d..3d3be00abf 100644
--- a/lib/Froxlor/UI/Listing.php
+++ b/lib/Froxlor/UI/Listing.php
@@ -25,10 +25,11 @@
 
 namespace Froxlor\UI;
 
 use Froxlor\CurrentUser;
 use Froxlor\Database\Database;
+use Froxlor\Froxlor;
 use Froxlor\UI\Panel\UI;
 use InvalidArgumentException;
 
 class Listing
 {
@@ -245,22 +246,38 @@ class Listing
 	 *         'column_name'
 	 *     ]
 	 * ]
 	 *
 	 * @param array $tabellisting
-	 * @return bool
+	 * @return array
 	 */
-	public static function storeColumnListingForUser(array $tabellisting): bool
+	public static function storeColumnListingForUser(array $tabellisting): array
 	{
 		$section = array_key_first($tabellisting);
 		if (empty($section) || !is_array($tabellisting[$section]) || empty($tabellisting[$section])) {
 			throw new InvalidArgumentException("Invalid selection array for " . __METHOD__);
 		}
 		$userid = 'customerid';
 		if (CurrentUser::isAdmin()) {
 			$userid = 'adminid';
 		}
+		// include all possible tablelisting-definitions to check for the right section
+		foreach(glob(Froxlor::getInstallDir().'lib/tablelisting/{,*/}*.php', GLOB_BRACE) as $tbl_file) {
+			$table_listings = include $tbl_file;
+			if (!isset($table_listings[$section])) {
+				continue;
+			} else {
+				break;
+			}
+		}
+		$columns_available = array_keys($table_listings[$section]['columns']);
+		// filter out unknown columns
+		foreach ($tabellisting[$section] as $index => $column_changed) {
+			if (!in_array($column_changed, $columns_available)) {
+				unset($tabellisting[$section][$index]);
+			}
+		}
 		// delete possible existing entry
 		self::deleteColumnListingForUser($tabellisting);
 		// add new entry
 		$ins_stmt = Database::prepare("
 			INSERT INTO `" . TABLE_PANEL_USERCOLUMNS . "` SET
@@ -271,11 +288,11 @@ class Listing
 		Database::pexecute($ins_stmt, [
 			'uid' => CurrentUser::getField($userid),
 			'section' => $section,
 			'cols' => json_encode($tabellisting[$section])
 		]);
-		return true;
+		return $tabellisting[$section];
 	}
 
 	/**
 	 * delete column listing selection of user from database
 	 *
diff --git a/lib/tablelisting/admin/tablelisting.domains.php b/lib/tablelisting/admin/tablelisting.domains.php
index d0fe248652..11f8ec5ed3 100644
--- a/lib/tablelisting/admin/tablelisting.domains.php
+++ b/lib/tablelisting/admin/tablelisting.domains.php
@@ -27,15 +27,18 @@ use Froxlor\UI\Callbacks\Domain;
 use Froxlor\UI\Callbacks\Impersonate;
 use Froxlor\UI\Callbacks\Style;
 use Froxlor\UI\Callbacks\Text;
 use Froxlor\UI\Listing;
 
+// used outside scope variables
+$customerCollectionCount = !is_null($customerCollection ?? null) ? $customerCollection->count() : 0;
+
 return [
 	'domain_list' => [
 		'title' => lng('admin.domains'),
 		'icon' => 'fa-solid fa-globe',
-		'empty_msg' => $customerCollection->count() == 0 ? lng('admin.domain_nocustomeraddingavailable') : '',
+		'empty_msg' => $customerCollectionCount == 0 ? lng('admin.domain_nocustomeraddingavailable') : '',
 		'self_overview' => ['section' => 'domains', 'page' => 'domains'],
 		'default_sorting' => ['d.domain_ace' => 'asc'],
 		'columns' => [
 			'd.id' => [
 				'label' => 'ID',
diff --git a/lib/tablelisting/customer/tablelisting.htaccess.php b/lib/tablelisting/customer/tablelisting.htaccess.php
index c29adf42ae..f2505d2844 100644
--- a/lib/tablelisting/customer/tablelisting.htaccess.php
+++ b/lib/tablelisting/customer/tablelisting.htaccess.php
@@ -25,10 +25,13 @@
 
 use Froxlor\UI\Callbacks\Ftp;
 use Froxlor\UI\Callbacks\Text;
 use Froxlor\UI\Listing;
 
+// used outside scope variables
+$cperlenabled = $cperlenabled ?? false;
+
 return [
 	'htaccess_list' => [
 		'title' => lng('menue.extras.pathoptions'),
 		'icon' => 'fa-solid fa-folder',
 		'self_overview' => ['section' => 'extras', 'page' => 'htaccess'],
diff --git a/lib/tablelisting/customer/tablelisting.mysqls.php b/lib/tablelisting/customer/tablelisting.mysqls.php
index dc599f3bb9..cc63d9f366 100644
--- a/lib/tablelisting/customer/tablelisting.mysqls.php
+++ b/lib/tablelisting/customer/tablelisting.mysqls.php
@@ -25,10 +25,13 @@
 
 use Froxlor\UI\Callbacks\Mysql;
 use Froxlor\UI\Callbacks\Text;
 use Froxlor\UI\Listing;
 
+// used outside scope variables
+$multiple_mysqlservers = $multiple_mysqlservers ?? false;
+
 return [
 	'mysql_list' => [
 		'title' => lng('menue.mysql.databases'),
 		'icon' => 'fa-solid fa-database',
 		'self_overview' => ['section' => 'mysql', 'page' => 'mysqls'],
diff --git a/lib/tablelisting/tablelisting.dns.php b/lib/tablelisting/tablelisting.dns.php
index 1f2ea9376c..293e08f42d 100644
--- a/lib/tablelisting/tablelisting.dns.php
+++ b/lib/tablelisting/tablelisting.dns.php
@@ -25,10 +25,14 @@
 
 use Froxlor\UI\Callbacks\Dns;
 use Froxlor\UI\Callbacks\Text;
 use Froxlor\UI\Listing;
 
+// used outside scope variables
+$domain = $domain ?? '';
+$domain_id = $domain_id ?? '';
+
 return [
 	'dns_list' => [
 		'title' => 'DNS Entries',
 		'description' => $domain,
 		'icon' => 'fa-solid fa-globe',

Vulnerable code: 		foreach ((Request::any('columns') ?? []) as $value) {
			Listing::storeColumnListingForUser([Request::any('listing') => $columns]);
		Listing::deleteColumnListingForUser([Request::any('listing') => []]);
	 * @return bool
	public static function storeColumnListingForUser(array $tabellisting): bool
		return true;
		'empty_msg' => $customerCollection->count() == 0 ? lng('admin.domain_nocustomeraddingavailable') : '',
Fixed code: 		foreach ((Request::post('columns') ?? []) as $value) {
			$columns = Listing::storeColumnListingForUser([Request::post('listing') => $columns]);
		Listing::deleteColumnListingForUser([Request::post('listing') => []]);
use Froxlor\Froxlor;
	 * @return array
	public static function storeColumnListingForUser(array $tabellisting): array
		// include all possible tablelisting-definitions to check for the right section
		foreach(glob(Froxlor::getInstallDir().'lib/tablelisting/{,*/}*.php', GLOB_BRACE) as $tbl_file) {
			$table_listings = include $tbl_file;
			if (!isset($table_listings[$section])) {
				continue;
			} else {
				break;
			}
		}
		$columns_available = array_keys($table_listings[$section]['columns']);
		// filter out unknown columns
		foreach ($tabellisting[$section] as $index => $column_changed) {
			if (!in_array($column_changed, $columns_available)) {
				unset($tabellisting[$section][$index]);
			}
		}
		return $tabellisting[$section];
// used outside scope variables
$customerCollectionCount = !is_null($customerCollection ?? null) ? $customerCollection->count() : 0;

		'empty_msg' => $customerCollectionCount == 0 ? lng('admin.domain_nocustomeraddingavailable') : '',
// used outside scope variables
$cperlenabled = $cperlenabled ?? false;

// used outside scope variables
$multiple_mysqlservers = $multiple_mysqlservers ?? false;

// used outside scope variables
$domain = $domain ?? '';
$domain_id = $domain_id ?? '';