From 64ea56164ad8f0f2cee5676f84d2d8f064e986e1 Mon Sep 17 00:00:00 2001
From: Marius Cramer <m.cramer@pixcept.de>
Date: Tue, 29 Jul 2014 11:55:13 -0400
Subject: [PATCH] Improved input validation

---
 server/plugins-available/shelluser_base_plugin.inc.php    |   14 +++++++
 server/plugins-available/shelluser_jailkit_plugin.inc.php |   14 +++++++
 server/plugins-available/apache2_plugin.inc.php           |    3 +
 server/plugins-available/cron_jailkit_plugin.inc.php      |   16 +++++--
 interface/web/sites/web_domain_edit.php                   |    4 +
 server/plugins-available/cron_plugin.inc.php              |    9 +++-
 server/plugins-available/nginx_plugin.inc.php             |    5 ++
 interface/lib/classes/functions.inc.php                   |   18 +++++++++
 server/lib/classes/system.inc.php                         |   26 ++++++++++++
 9 files changed, 97 insertions(+), 12 deletions(-)

diff --git a/interface/lib/classes/functions.inc.php b/interface/lib/classes/functions.inc.php
index 5f62f52..ddee8da 100644
--- a/interface/lib/classes/functions.inc.php
+++ b/interface/lib/classes/functions.inc.php
@@ -424,6 +424,24 @@
 		return implode("\n", $domains);
 	}
 
+	public function is_allowed_user($username, $restrict_names = false) {
+		global $app;
+		
+		if($username == 'root') return false;
+		if($restrict_names == true && preg_match('/^web\d+$/', $username) == false) return false;
+		
+		return true;
+	}
+	
+	public function is_allowed_group($groupname, $restrict_names = false) {
+		global $app;
+		
+		if($groupname == 'root') return false;
+		if($restrict_names == true && preg_match('/^client\d+$/', $groupname) == false) return false;
+		
+		return true;
+	}
+
 }
 
 ?>
diff --git a/interface/web/sites/web_domain_edit.php b/interface/web/sites/web_domain_edit.php
index 8edfffa..1208e48 100644
--- a/interface/web/sites/web_domain_edit.php
+++ b/interface/web/sites/web_domain_edit.php
@@ -607,9 +607,11 @@
 			// When the record is updated
 			if($this->id > 0) {
 				// restore the server ID if the user is not admin and record is edited
-				$tmp = $app->db->queryOneRecord("SELECT server_id, `cgi`, `ssi`, `perl`, `ruby`, `python`, `suexec`, `errordocs`, `subdomain`, `ssl` FROM web_domain WHERE domain_id = ".$app->functions->intval($this->id));
+				$tmp = $app->db->queryOneRecord("SELECT server_id, `system_user`, `system_group`, `cgi`, `ssi`, `perl`, `ruby`, `python`, `suexec`, `errordocs`, `subdomain`, `ssl` FROM web_domain WHERE domain_id = ".$app->functions->intval($this->id));
 				$this->dataRecord["server_id"] = $tmp["server_id"];
 
+				$this->dataRecord['system_user'] = $tmp['system_user'];
+				$this->dataRecord['system_group'] = $tmp['system_group'];
 				// set the settings to current if not provided (or cleared due to limits)
 				if($this->dataRecord['cgi'] == 'n') $this->dataRecord['cgi'] = $tmp['cgi'];
 				if($this->dataRecord['ssi'] == 'n') $this->dataRecord['ssi'] = $tmp['ssi'];
diff --git a/server/lib/classes/system.inc.php b/server/lib/classes/system.inc.php
index d9d85e5..049eb61 100644
--- a/server/lib/classes/system.inc.php
+++ b/server/lib/classes/system.inc.php
@@ -34,7 +34,9 @@
 	var $server_id;
 	var $server_conf;
 	var $data;
-
+	var $min_uid = 500;
+	var $min_gid = 500;
+	
 	/**
 	 * Construct for this class
 	 *
@@ -1816,6 +1818,28 @@
 		return true;
 	}
 	
+	public function is_allowed_user($username, $check_id = true, $restrict_names = false) {
+		global $app;
+		
+		if($username == 'root') return false;
+		if($check_id && intval($this->getuid($username)) < $this->min_uid) return false;
+		
+		if($restrict_names == true && preg_match('/^web\d+$/', $username) == false) return false;
+		
+		return true;
+	}
+	
+	public function is_allowed_group($groupname, $restrict_names = false) {
+		global $app;
+		
+		if($groupname == 'root') return false;
+		if(intval($this->getgid($groupname)) < $this->min_gid) return false;
+		
+		if($restrict_names == true && preg_match('/^client\d+$/', $groupname) == false) return false;
+		
+		return true;
+	}
+	
 }
 
 ?>
diff --git a/server/plugins-available/apache2_plugin.inc.php b/server/plugins-available/apache2_plugin.inc.php
index addcd13..b1411c9 100644
--- a/server/plugins-available/apache2_plugin.inc.php
+++ b/server/plugins-available/apache2_plugin.inc.php
@@ -344,7 +344,8 @@
 			if($data['new']['type'] == 'vhost' || $data['new']['type'] == 'vhostsubdomain') $app->log('document_root not set', LOGLEVEL_WARN);
 			return 0;
 		}
-		if($data['new']['system_user'] == 'root' or $data['new']['system_group'] == 'root') {
+		if(!$app->system->is_allowed_user($data['new']['system_user'], false, true)
+			|| !$app->system->is_allowed_group($data['new']['system_group'], false, true)) {
 			$app->log('Websites cannot be owned by the root user or group.', LOGLEVEL_WARN);
 			return 0;
 		}
diff --git a/server/plugins-available/cron_jailkit_plugin.inc.php b/server/plugins-available/cron_jailkit_plugin.inc.php
index c3bd5b7..4c95b83 100644
--- a/server/plugins-available/cron_jailkit_plugin.inc.php
+++ b/server/plugins-available/cron_jailkit_plugin.inc.php
@@ -80,10 +80,14 @@
 		if(!$parent_domain["domain_id"]) {
 			$app->log("Parent domain not found", LOGLEVEL_WARN);
 			return 0;
-		} elseif($parent_domain["system_user"] == 'root' or $parent_domain["system_group"] == 'root') {
-			$app->log("Websites (and Crons) cannot be owned by the root user or group.", LOGLEVEL_WARN);
-			return 0;
 		}
+
+		if(!$app->system->is_allowed_user($parent_domain['system_user'], true, true)
+			|| !$app->system->is_allowed_group($parent_domain['system_group'], true, true)) {
+			$app->log("Websites (and Crons) cannot be owned by the root user or group.", LOGLEVEL_WARN);
+			return false;
+		}
+
 
 		$this->parent_domain = $parent_domain;
 
@@ -155,9 +159,11 @@
 		if(!$parent_domain["domain_id"]) {
 			$app->log("Parent domain not found", LOGLEVEL_WARN);
 			return 0;
-		} elseif($parent_domain["system_user"] == 'root' or $parent_domain["system_group"] == 'root') {
+		}
+		if(!$app->system->is_allowed_user($parent_domain['system_user'], true, true)
+			|| !$app->system->is_allowed_group($parent_domain['system_group'], true, true)) {
 			$app->log("Websites (and Crons) cannot be owned by the root user or group.", LOGLEVEL_WARN);
-			return 0;
+			return false;
 		}
 
 		$app->uses('system');
diff --git a/server/plugins-available/cron_plugin.inc.php b/server/plugins-available/cron_plugin.inc.php
index fe00713..7f8070d 100644
--- a/server/plugins-available/cron_plugin.inc.php
+++ b/server/plugins-available/cron_plugin.inc.php
@@ -96,11 +96,14 @@
 		if(!$parent_domain["domain_id"]) {
 			$app->log("Parent domain not found", LOGLEVEL_WARN);
 			return 0;
-		} elseif($parent_domain["system_user"] == 'root' or $parent_domain["system_group"] == 'root') {
-			$app->log("Websites (and Crons) cannot be owned by the root user or group.", LOGLEVEL_WARN);
-			return 0;
 		}
 
+		if(!$app->system->is_allowed_user($parent_domain['system_user'], true, true)
+			|| !$app->system->is_allowed_group($parent_domain['system_group'], true, true)) {
+			$app->log("Websites (and Crons) cannot be owned by the root user or group.", LOGLEVEL_WARN);
+			return false;
+		}
+		
 		// Get the client ID
 		$client = $app->dbmaster->queryOneRecord("SELECT client_id FROM sys_group WHERE sys_group.groupid = ".intval($data["new"]["sys_groupid"]));
 		$client_id = intval($client["client_id"]);
diff --git a/server/plugins-available/nginx_plugin.inc.php b/server/plugins-available/nginx_plugin.inc.php
index d83ecf4..2837e0e 100644
--- a/server/plugins-available/nginx_plugin.inc.php
+++ b/server/plugins-available/nginx_plugin.inc.php
@@ -351,10 +351,13 @@
 			if($data['new']['type'] == 'vhost' || $data['new']['type'] == 'vhostsubdomain') $app->log('document_root not set', LOGLEVEL_WARN);
 			return 0;
 		}
-		if($data['new']['system_user'] == 'root' or $data['new']['system_group'] == 'root') {
+
+		if(!$app->system->is_allowed_user($data['new']['system_user'], false, true)
+			|| !$app->system->is_allowed_group($data['new']['system_group'], false, true)) {
 			$app->log('Websites cannot be owned by the root user or group.', LOGLEVEL_WARN);
 			return 0;
 		}
+
 		if(trim($data['new']['domain']) == '') {
 			$app->log('domain is empty', LOGLEVEL_WARN);
 			return 0;
diff --git a/server/plugins-available/shelluser_base_plugin.inc.php b/server/plugins-available/shelluser_base_plugin.inc.php
index 0ceced9..e331624 100755
--- a/server/plugins-available/shelluser_base_plugin.inc.php
+++ b/server/plugins-available/shelluser_base_plugin.inc.php
@@ -82,6 +82,13 @@
 			$app->log('Directory of the shell user is not valid.',LOGLEVEL_WARN);
 			return false;
 		}
+		
+		if(!$app->system->is_allowed_user($data['new']['username'], false, false)
+			|| !$app->system->is_allowed_user($data['new']['puser'], true, true)
+			|| !$app->system->is_allowed_group($data['new']['pgroup'], true, true)) {
+			$app->log('Shell user must not be root or in group root.',LOGLEVEL_WARN);
+			return false;
+		}
 
 		if($app->system->is_user($data['new']['puser'])) {
 
@@ -151,6 +158,13 @@
 			return false;
 		}
 
+		if(!$app->system->is_allowed_user($data['new']['username'], false, false)
+			|| !$app->system->is_allowed_user($data['new']['puser'], true, true)
+			|| !$app->system->is_allowed_group($data['new']['pgroup'], true, true)) {
+			$app->log('Shell user must not be root or in group root.',LOGLEVEL_WARN);
+			return false;
+		}
+		
 		if($app->system->is_user($data['new']['puser'])) {
 			// Get the UID of the parent user
 			$uid = intval($app->system->getuid($data['new']['puser']));
diff --git a/server/plugins-available/shelluser_jailkit_plugin.inc.php b/server/plugins-available/shelluser_jailkit_plugin.inc.php
index 90ed677..9cf6fc8 100755
--- a/server/plugins-available/shelluser_jailkit_plugin.inc.php
+++ b/server/plugins-available/shelluser_jailkit_plugin.inc.php
@@ -74,6 +74,13 @@
 		$app->uses('system');
 		$web = $app->db->queryOneRecord("SELECT * FROM web_domain WHERE domain_id = ".$data['new']['parent_domain_id']);
 
+		if(!$app->system->is_allowed_user($data['new']['username'], false, false)
+			|| !$app->system->is_allowed_user($data['new']['puser'], true, true)
+			|| !$app->system->is_allowed_group($data['new']['pgroup'], true, true)) {
+			$app->log('Shell user must not be root or in group root.',LOGLEVEL_WARN);
+			return false;
+		}
+
 		if($app->system->is_user($data['new']['puser'])) {
 			// Get the UID of the parent user
 			$uid = intval($app->system->getuid($data['new']['puser']));
@@ -139,6 +146,13 @@
 		$app->uses('system');
 		$web = $app->db->queryOneRecord("SELECT * FROM web_domain WHERE domain_id = ".$data['new']['parent_domain_id']);
 
+		if(!$app->system->is_allowed_user($data['new']['username'], false, false)
+			|| !$app->system->is_allowed_user($data['new']['puser'], true, true)
+			|| !$app->system->is_allowed_group($data['new']['pgroup'], true, true)) {
+			$app->log('Shell user must not be root or in group root.',LOGLEVEL_WARN);
+			return false;
+		}
+
 		if($app->system->is_user($data['new']['puser'])) {
 			// Get the UID of the parent user
 			$uid = intval($app->system->getuid($data['new']['puser']));

--
Gitblit v1.9.1