fix incorrect security check on mail-directories where various special-characters are allowed, fixes #1458

Signed-off-by: Michael Kaufmann (d00p) <d00p@froxlor.org>
This commit is contained in:
Michael Kaufmann (d00p)
2014-10-01 07:29:25 +02:00
parent cbab67a2fd
commit 480e3a8bfd
3 changed files with 30 additions and 19 deletions

View File

@@ -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) {
$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, ';'))
|| (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");
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

View File

@@ -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);
}

View File

@@ -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('|', '&', '`', '$', '~', '?'));
}
}
}