From 64ea56164ad8f0f2cee5676f84d2d8f064e986e1 Mon Sep 17 00:00:00 2001 From: Marius Cramer Date: Tue, 29 Jul 2014 17:55:13 +0200 Subject: [PATCH] Improved input validation --- interface/lib/classes/functions.inc.php | 18 +++++++++++++ interface/web/sites/web_domain_edit.php | 4 ++- server/lib/classes/system.inc.php | 26 ++++++++++++++++++- .../plugins-available/apache2_plugin.inc.php | 3 ++- .../cron_jailkit_plugin.inc.php | 14 +++++++--- server/plugins-available/cron_plugin.inc.php | 9 ++++--- server/plugins-available/nginx_plugin.inc.php | 5 +++- .../shelluser_base_plugin.inc.php | 14 ++++++++++ .../shelluser_jailkit_plugin.inc.php | 14 ++++++++++ 9 files changed, 96 insertions(+), 11 deletions(-) diff --git a/interface/lib/classes/functions.inc.php b/interface/lib/classes/functions.inc.php index 5f62f52ec..ddee8da9f 100644 --- a/interface/lib/classes/functions.inc.php +++ b/interface/lib/classes/functions.inc.php @@ -424,6 +424,24 @@ class functions { 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 8edfffafb..1208e48a1 100644 --- a/interface/web/sites/web_domain_edit.php +++ b/interface/web/sites/web_domain_edit.php @@ -607,9 +607,11 @@ class page_action extends tform_actions { // 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 d9d85e5a3..049eb618c 100644 --- a/server/lib/classes/system.inc.php +++ b/server/lib/classes/system.inc.php @@ -34,7 +34,9 @@ class system{ var $server_id; var $server_conf; var $data; - + var $min_uid = 500; + var $min_gid = 500; + /** * Construct for this class * @@ -1816,6 +1818,28 @@ class system{ 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 addcd138b..b1411c9f0 100644 --- a/server/plugins-available/apache2_plugin.inc.php +++ b/server/plugins-available/apache2_plugin.inc.php @@ -344,7 +344,8 @@ class apache2_plugin { 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 c3bd5b749..4c95b83c2 100644 --- a/server/plugins-available/cron_jailkit_plugin.inc.php +++ b/server/plugins-available/cron_jailkit_plugin.inc.php @@ -80,11 +80,15 @@ class cron_jailkit_plugin { 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; } + $this->parent_domain = $parent_domain; $app->uses('system'); @@ -155,9 +159,11 @@ class cron_jailkit_plugin { 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 fe00713ec..7f8070d78 100644 --- a/server/plugins-available/cron_plugin.inc.php +++ b/server/plugins-available/cron_plugin.inc.php @@ -96,11 +96,14 @@ class cron_plugin { 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 d83ecf442..2837e0eb9 100644 --- a/server/plugins-available/nginx_plugin.inc.php +++ b/server/plugins-available/nginx_plugin.inc.php @@ -351,10 +351,13 @@ class nginx_plugin { 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 0ceced99d..e33162413 100755 --- a/server/plugins-available/shelluser_base_plugin.inc.php +++ b/server/plugins-available/shelluser_base_plugin.inc.php @@ -82,6 +82,13 @@ class shelluser_base_plugin { $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 @@ class shelluser_base_plugin { 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 90ed6777f..9cf6fc89d 100755 --- a/server/plugins-available/shelluser_jailkit_plugin.inc.php +++ b/server/plugins-available/shelluser_jailkit_plugin.inc.php @@ -74,6 +74,13 @@ class shelluser_jailkit_plugin { $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 @@ class shelluser_jailkit_plugin { $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'])); -- GitLab