Skip to content

Commit 5b49c92

Browse files
author
Till Brehm
committed
Merge branch '6822-extend-server-side-cron-checks' into 'develop'
Resolve "Extend server-side cron checks" Closes #6822 See merge request ispconfig/ispconfig3!1975
2 parents aade24f + d28d0bb commit 5b49c92

File tree

3 files changed

+21
-5
lines changed

3 files changed

+21
-5
lines changed

interface/lib/classes/validate_cron.inc.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ function command_format($field_name, $field_value, $validator) {
5353
if($parsed["scheme"] != "http" && $parsed["scheme"] != "https") return $this->get_error($validator['errmsg']);
5454

5555
if(preg_match("'^([a-z0-9][a-z0-9_\-]{0,62}\.)+([A-Za-z0-9\-]{2,63})$'i", $parsed["host"]) == false) return $this->get_error($validator['errmsg']);
56+
57+
if(strpos($field_value, '\\') !== false) {
58+
return $this->get_error($validator['errmsg']);
59+
}
5660
}
5761
if(strpos($field_value, "\n") !== false || strpos($field_value, "\r") !== false || strpos($field_value, chr(0)) !== false) {
5862
return $this->get_error($validator['errmsg']);

server/plugins-available/cron_jailkit_plugin.inc.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ function insert($event_name, $data) {
121121

122122
$this->_add_jailkit_user();
123123

124-
$command .= 'usermod -U ? 2>/dev/null';
124+
$command = 'usermod -U ? 2>/dev/null';
125125
$app->system->exec_safe($command, $parent_domain["system_user"]);
126126

127127
$this->_update_website_security_level();

server/plugins-available/cron_plugin.inc.php

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,12 @@ function _write_crontab() {
219219
$cmd_count = 0;
220220
$chr_cmd_count = 0;
221221

222+
// Check if parentDomain array is empty
223+
if(!is_array($this->parent_domain) || count($this->parent_domain) == 0) {
224+
$app->log("Parent domain not found", LOGLEVEL_WARN);
225+
return 0;
226+
}
227+
222228
//* read all active cron jobs from database and write them to file
223229
$cron_jobs = $app->db->queryAllRecords("SELECT c.`run_min`, c.`run_hour`, c.`run_mday`, c.`run_month`, c.`run_wday`, c.`command`, c.`type`, c.`log`, `web_domain`.`domain` as `domain` FROM `cron` as c INNER JOIN `web_domain` ON `web_domain`.`domain_id` = c.`parent_domain_id` WHERE c.`parent_domain_id` = ? AND c.`active` = 'y'", $this->parent_domain["domain_id"]);
224230
if($cron_jobs && count($cron_jobs) > 0) {
@@ -240,15 +246,21 @@ function _write_crontab() {
240246
$log_wget_target = $log_root . '/cron_wget.log';
241247
}
242248

249+
// Check if command contains invalid chars
250+
if(strpos($job['command'], "\n") !== false || strpos($job['command'], "\r") !== false || strpos($job['command'], chr(0)) !== false) {
251+
$app->log("Insecure Cron job SKIPPED: " . $job['command'], LOGLEVEL_WARN);
252+
continue;
253+
}
254+
243255
$cron_line .= "\t{$this->parent_domain['system_user']}"; //* running as user
244256
if($job['type'] == 'url') {
245-
$cron_line .= "\t{$cron_config['wget']} --no-check-certificate --user-agent='Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0' -q -t 1 -T 7200 -O " . $log_wget_target . " " . escapeshellarg($job['command']) . " " . $log_target;
246-
} else {
247-
if(strpos($job['command'], "\n") !== false || strpos($job['command'], "\r") !== false || strpos($job['command'], chr(0)) !== false) {
257+
// Check that command does not contain a backslash
258+
if (strpos($job['command'], '\\') !== false) {
248259
$app->log("Insecure Cron job SKIPPED: " . $job['command'], LOGLEVEL_WARN);
249260
continue;
250261
}
251-
262+
$cron_line .= "\t{$cron_config['wget']} --no-check-certificate --user-agent='Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:47.0) Gecko/20100101 Firefox/47.0' -q -t 1 -T 7200 -O " . $log_wget_target . " " . escapeshellarg($job['command']) . " " . $log_target;
263+
} else {
252264
$web_root = '';
253265
if($job['type'] == 'chrooted') {
254266
if(substr($job['command'], 0, strlen($this->parent_domain['document_root'])) == $this->parent_domain['document_root']) {

0 commit comments

Comments
 (0)