From 480e3a8bfd39f69267158f96ff59aed918ea921b Mon Sep 17 00:00:00 2001 From: "Michael Kaufmann (d00p)" Date: Wed, 1 Oct 2014 07:29:25 +0200 Subject: [PATCH] fix incorrect security check on mail-directories where various special-characters are allowed, fixes #1458 Signed-off-by: Michael Kaufmann (d00p) --- lib/functions/filedir/function.safe_exec.php | 33 +++++++++++--------- scripts/jobs/cron_mailboxsize.php | 4 ++- scripts/jobs/cron_tasks.php | 12 +++++-- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/lib/functions/filedir/function.safe_exec.php b/lib/functions/filedir/function.safe_exec.php index 012998cc..98f765fc 100644 --- a/lib/functions/filedir/function.safe_exec.php +++ b/lib/functions/filedir/function.safe_exec.php @@ -20,24 +20,27 @@ /** * Wrapper around the exec command. * - * @param string exec_string String to be executed + * @param string $exec_string command to be executed + * @param string $return_value referenced variable where the output is stored + * @param array $allowedChars optional array of allowed characters in path/command * - * @return string The result of the exec() + * @return string result of exec() */ -function safe_exec($exec_string, &$return_value = false) { +function safe_exec($exec_string, &$return_value = false, $allowedChars = null) { - // check for bad signs in execute command - if ((stristr($exec_string, ';')) - || (stristr($exec_string, '|')) - || (stristr($exec_string, '&')) - || (stristr($exec_string, '>')) - || (stristr($exec_string, '<')) - || (stristr($exec_string, '`')) - || (stristr($exec_string, '$')) - || (stristr($exec_string, '~')) - || (stristr($exec_string, '?')) - ) { - die('SECURITY CHECK FAILED!' . "\n" . 'The execute string "' . htmlspecialchars($exec_string) . '" is a possible security risk!' . "\n" . 'Please check your whole server for security problems by hand!' . "\n"); + $disallowed = array(';', '|', '&', '>', '<', '`', '$', '~', '?'); + + $acheck = false; + if ($allowedChars != null && is_array($allowedChars) && count($allowedChars) > 0) { + $acheck = true; + } + + foreach ($disallowed as $dc) { + if ($acheck && in_array($dc, $allowedChars)) continue; + // check for bad signs in execute command + if (stristr($exec_string, $dc)) { + die("SECURITY CHECK FAILED!\nThe execute string '" . $exec_string . "' is a possible security risk!\nPlease check your whole server for security problems by hand!\n"); + } } // execute the command and return output diff --git a/scripts/jobs/cron_mailboxsize.php b/scripts/jobs/cron_mailboxsize.php index c5b46567..e4737f03 100644 --- a/scripts/jobs/cron_mailboxsize.php +++ b/scripts/jobs/cron_mailboxsize.php @@ -35,7 +35,9 @@ while ($maildir = $maildirs_stmt->fetch(PDO::FETCH_ASSOC)) { if (file_exists($_maildir) && is_dir($_maildir) ) { - $back = safe_exec('du -sk ' . escapeshellarg($_maildir) . ''); + // mail-adress allows many special characters, see http://en.wikipedia.org/wiki/Email_address#Local_part + $return = false; + $back = safe_exec('du -sk ' . escapeshellarg($_maildir), $return, array('|', '&', '`', '$', '~', '?')); foreach ($back as $backrow) { $emailusage = explode(' ', $backrow); } diff --git a/scripts/jobs/cron_tasks.php b/scripts/jobs/cron_tasks.php index dc46a23d..2d8bcb38 100644 --- a/scripts/jobs/cron_tasks.php +++ b/scripts/jobs/cron_tasks.php @@ -210,7 +210,9 @@ while ($row = $result_tasks_stmt->fetch(PDO::FETCH_ASSOC)) { && filegroup($maildir) == Settings::Get('system.vmail_gid') ) { $cronlog->logAction(CRON_ACTION, LOG_NOTICE, 'Running: rm -rf ' . escapeshellarg($maildir)); - safe_exec('rm -rf '.escapeshellarg($maildir)); + // mail-adress allows many special characters, see http://en.wikipedia.org/wiki/Email_address#Local_part + $return = false; + safe_exec('rm -rf '.escapeshellarg($maildir), $return, array('|', '&', '`', '$', '~', '?')); } // remove tmpdir if it exists @@ -281,7 +283,9 @@ while ($row = $result_tasks_stmt->fetch(PDO::FETCH_ASSOC)) { && filegroup($maildir) == Settings::Get('system.vmail_gid') ) { $cronlog->logAction(CRON_ACTION, LOG_NOTICE, 'Running: rm -rf ' . escapeshellarg($maildir)); - safe_exec('rm -rf '.escapeshellarg($maildir)); + // mail-adress allows many special characters, see http://en.wikipedia.org/wiki/Email_address#Local_part + $return = false; + safe_exec('rm -rf '.escapeshellarg($maildir), $return, array('|', '&', '`', '$', '~', '?')); } else { // backward-compatibility for old folder-structure @@ -296,7 +300,9 @@ while ($row = $result_tasks_stmt->fetch(PDO::FETCH_ASSOC)) { && filegroup($maildir_old) == Settings::Get('system.vmail_gid') ) { $cronlog->logAction(CRON_ACTION, LOG_NOTICE, 'Running: rm -rf ' . escapeshellarg($maildir_old)); - safe_exec('rm -rf '.escapeshellarg($maildir_old)); + // mail-adress allows many special characters, see http://en.wikipedia.org/wiki/Email_address#Local_part + $return = false; + safe_exec('rm -rf '.escapeshellarg($maildir_old), $return, array('|', '&', '`', '$', '~', '?')); } } }