From 037af6890fe6fdb84a08d3c86083e847c90ec0ad Mon Sep 17 00:00:00 2001 From: Aleksander Machniak <alec@alec.pl> Date: Tue, 22 Oct 2013 08:17:26 -0400 Subject: [PATCH] Fix vulnerability in handling _session argument of utils/save-prefs (#1489382) --- plugins/filesystem_attachments/filesystem_attachments.php | 53 ++++++++++++++++++++++++++++++----------------------- 1 files changed, 30 insertions(+), 23 deletions(-) diff --git a/plugins/filesystem_attachments/filesystem_attachments.php b/plugins/filesystem_attachments/filesystem_attachments.php index d5f5553..d952e5a 100644 --- a/plugins/filesystem_attachments/filesystem_attachments.php +++ b/plugins/filesystem_attachments/filesystem_attachments.php @@ -1,7 +1,7 @@ <?php /** * Filesystem Attachments - * + * * This is a core plugin which provides basic, filesystem based * attachment temporary file handling. This includes storing * attachments of messages currently being composed, writing attachments @@ -15,32 +15,32 @@ * * @author Ziba Scott <ziba@umich.edu> * @author Thomas Bruederli <roundcube@gmail.com> - * + * */ class filesystem_attachments extends rcube_plugin { - public $task = 'mail'; - + public $task = '?(?!login).*'; + function init() { // Save a newly uploaded attachment - $this->add_hook('upload_attachment', array($this, 'upload')); + $this->add_hook('attachment_upload', array($this, 'upload')); // Save an attachment from a non-upload source (draft or forward) - $this->add_hook('save_attachment', array($this, 'save')); + $this->add_hook('attachment_save', array($this, 'save')); // Remove an attachment from storage - $this->add_hook('remove_attachment', array($this, 'remove')); + $this->add_hook('attachment_delete', array($this, 'remove')); // When composing an html message, image attachments may be shown - $this->add_hook('display_attachment', array($this, 'display')); + $this->add_hook('attachment_display', array($this, 'display')); // Get the attachment from storage and place it on disk to be sent - $this->add_hook('get_attachment', array($this, 'get_attachment')); + $this->add_hook('attachment_get', array($this, 'get')); // Delete all temp files associated with this user - $this->add_hook('cleanup_attachments', array($this, 'cleanup')); - $this->add_hook('kill_session', array($this, 'cleanup')); + $this->add_hook('attachments_cleanup', array($this, 'cleanup')); + $this->add_hook('session_destroy', array($this, 'cleanup')); } /** @@ -49,6 +49,7 @@ function upload($args) { $args['status'] = false; + $group = $args['group']; $rcmail = rcmail::get_instance(); // use common temp dir for file uploads @@ -59,9 +60,10 @@ $args['id'] = $this->file_id(); $args['path'] = $tmpfname; $args['status'] = true; + @chmod($tmpfname, 0600); // set correct permissions (#1488996) // Note the file for later cleanup - $_SESSION['plugins']['filesystem_attachments']['tmp_files'][] = $tmpfname; + $_SESSION['plugins']['filesystem_attachments'][$group][] = $tmpfname; } return $args; @@ -72,6 +74,7 @@ */ function save($args) { + $group = $args['group']; $args['status'] = false; if (!$args['path']) { @@ -86,12 +89,12 @@ } else return $args; } - + $args['id'] = $this->file_id(); $args['status'] = true; - + // Note the file for later cleanup - $_SESSION['plugins']['filesystem_attachments']['tmp_files'][] = $args['path']; + $_SESSION['plugins']['filesystem_attachments'][$group][] = $args['path']; return $args; } @@ -122,11 +125,11 @@ * on disk for use. This stub function is kept here to make this * class handy as a parent class for other plugins which may need it. */ - function get_attachment($args) + function get($args) { return $args; } - + /** * Delete all temp files associated with this user */ @@ -135,13 +138,17 @@ // $_SESSION['compose']['attachments'] is not a complete record of // temporary files because loading a draft or starting a forward copies // the file to disk, but does not make an entry in that array - if (is_array($_SESSION['plugins']['filesystem_attachments']['tmp_files'])){ - foreach ($_SESSION['plugins']['filesystem_attachments']['tmp_files'] as $filename){ - if(file_exists($filename)){ - unlink($filename); + if (is_array($_SESSION['plugins']['filesystem_attachments'])){ + foreach ($_SESSION['plugins']['filesystem_attachments'] as $group => $files) { + if ($args['group'] && $args['group'] != $group) + continue; + foreach ((array)$files as $filename){ + if(file_exists($filename)){ + unlink($filename); + } } + unset($_SESSION['plugins']['filesystem_attachments'][$group]); } - unset($_SESSION['plugins']['filesystem_attachments']['tmp_files']); } return $args; } @@ -149,7 +156,7 @@ function file_id() { $userid = rcmail::get_instance()->user->ID; - list($usec, $sec) = explode(' ', microtime()); + list($usec, $sec) = explode(' ', microtime()); return preg_replace('/[^0-9]/', '', $userid . $sec . $usec); } } -- Gitblit v1.9.1