From 2a32f51c91d5e9c7b1a9d931846dd44c008ff36d Mon Sep 17 00:00:00 2001 From: Aleksander Machniak Date: Wed, 8 Nov 2017 11:01:16 +0100 Subject: [PATCH 1/3] Fix file disclosure vulnerability caused by insuficient input validation in relation with attachment plugins (#6026) --- .../database_attachments.php | 7 ++- .../filesystem_attachments.php | 47 +++++++++++++++++-- program/include/rcmail.php | 5 +- 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/plugins/database_attachments/database_attachments.php b/plugins/database_attachments/database_attachments.php index 91335d4cf..5e3268d80 100644 --- a/plugins/database_attachments/database_attachments.php +++ b/plugins/database_attachments/database_attachments.php @@ -84,6 +84,8 @@ class database_attachments extends filesystem_attachments if ($args['data'] === false) { return $args; } + + $args['path'] = null; } $data = base64_encode($args['data']); @@ -130,10 +132,13 @@ class database_attachments extends filesystem_attachments $cache = $this->get_cache(); $data = $cache->read($args['id']); - if ($data) { + if ($data !== null && $data !== false) { $args['data'] = base64_decode($data); $args['status'] = true; } + else { + $args['status'] = false; + } return $args; } diff --git a/plugins/filesystem_attachments/filesystem_attachments.php b/plugins/filesystem_attachments/filesystem_attachments.php index 4bca47e29..7501e5533 100644 --- a/plugins/filesystem_attachments/filesystem_attachments.php +++ b/plugins/filesystem_attachments/filesystem_attachments.php @@ -7,12 +7,19 @@ * attachments of messages currently being composed, writing attachments * to disk when drafts with attachments are re-opened and writing * attachments to disk for inline display in current html compositions. + * It also handles uploaded files for other uses, so not only attachments. * * Developers may wish to extend this class when creating attachment * handler plugins: * require_once('plugins/filesystem_attachments/filesystem_attachments.php'); * class myCustom_attachments extends filesystem_attachments * + * Note for developers: It is plugin's responsibility to care about security. + * So, e.g. if the plugin is asked about some file path it should check + * if it's really the storage path of the plugin and not e.g. /etc/passwd. + * It is done by setting 'status' flag on every plugin hook it uses. + * Roundcube core will trust the returned path if status=true. + * * @license GNU GPLv3+ * @author Ziba Scott * @author Thomas Bruederli @@ -107,7 +114,7 @@ class filesystem_attachments extends rcube_plugin */ function remove($args) { - $args['status'] = @unlink($args['path']); + $args['status'] = $this->verify_path($args['path']) && @unlink($args['path']); return $args; } @@ -118,7 +125,7 @@ class filesystem_attachments extends rcube_plugin */ function display($args) { - $args['status'] = file_exists($args['path']); + $args['status'] = $this->verify_path($args['path']) && file_exists($args['path']); return $args; } @@ -129,6 +136,10 @@ class filesystem_attachments extends rcube_plugin */ function get($args) { + if (!$this->verify_path($args['path'])) { + $args['path'] = null; + } + return $args; } @@ -147,7 +158,7 @@ class filesystem_attachments extends rcube_plugin } foreach ((array)$files as $filename) { - if(file_exists($filename)) { + if (file_exists($filename)) { unlink($filename); } } @@ -182,4 +193,34 @@ class filesystem_attachments extends rcube_plugin } } } + + /** + * For security we'll always verify the file path stored in session, + * as session entries can be faked in various ways e.g. #6026. + * We allow only files in Roundcube temp dir + */ + protected function verify_path($path) + { + if (empty($path)) { + return false; + } + + $rcmail = rcube::get_instance(); + $temp_dir = $rcmail->config->get('temp_dir'); + $file_path = pathinfo($path, PATHINFO_DIRNAME); + + if ($temp_dir !== $file_path) { + rcube::raise_error(array( + 'code' => 403, + 'file' => __FILE__, + 'line' => __LINE__, + 'message' => sprintf("%s can't read %s (not in temp_dir)", + $rcmail->get_user_name(), substr($path, 0, 512)) + ), true, false); + + return false; + } + + return true; + } } diff --git a/program/include/rcmail.php b/program/include/rcmail.php index 3a96352e1..1bdda0d8c 100644 --- a/program/include/rcmail.php +++ b/program/include/rcmail.php @@ -678,8 +678,9 @@ class rcmail extends rcube $_SESSION['password'] = $this->encrypt($password); $_SESSION['login_time'] = time(); - if (isset($_REQUEST['_timezone']) && $_REQUEST['_timezone'] != '_default_') { - $_SESSION['timezone'] = rcube_utils::get_input_value('_timezone', rcube_utils::INPUT_GPC); + $timezone = rcube_utils::get_input_value('_timezone', rcube_utils::INPUT_GPC); + if ($timezone && is_string($timezone) && $timezone != '_default_') { + $_SESSION['timezone'] = $timezone; } // fix some old settings according to namespace prefix From 968e20c5e5624a1df5dea2fcd792b1f46add3d1a Mon Sep 17 00:00:00 2001 From: Aleksander Machniak Date: Wed, 8 Nov 2017 11:03:29 +0100 Subject: [PATCH 2/3] Update changelog --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 1f2e11665..c6637e47c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -52,6 +52,7 @@ CHANGELOG Roundcube Webmail - Fix various issues when downloading files with names containing non-ascii chars, use RFC 2231 (#5772) - Fix decoding of mailto: links with + character in HTML messages (#6020) - Fix false reporting of failed upgrade in installto.sh (#6019) +- Fix file disclosure vulnerability caused by insuficient input validation in relation to attachment plugins (#6026) RELEASE 1.3.2 ------------- From a0374f3c45578b8b51ff1850c00f4f0732e17193 Mon Sep 17 00:00:00 2001 From: Aleksander Machniak Date: Wed, 8 Nov 2017 12:38:19 +0100 Subject: [PATCH 3/3] Fix mangled non-ASCII characters in links in HTML messages (#6028) --- CHANGELOG | 1 + program/lib/Roundcube/html.php | 5 ++++- tests/Framework/Html.php | 4 ++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index c6637e47c..2d7f74bd9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -53,6 +53,7 @@ CHANGELOG Roundcube Webmail - Fix decoding of mailto: links with + character in HTML messages (#6020) - Fix false reporting of failed upgrade in installto.sh (#6019) - Fix file disclosure vulnerability caused by insuficient input validation in relation to attachment plugins (#6026) +- Fix mangled non-ASCII characters in links in HTML messages (#6028) RELEASE 1.3.2 ------------- diff --git a/program/lib/Roundcube/html.php b/program/lib/Roundcube/html.php index ffb049301..eea6a2f04 100644 --- a/program/lib/Roundcube/html.php +++ b/program/lib/Roundcube/html.php @@ -349,7 +349,10 @@ class html public static function parse_attrib_string($str) { $attrib = array(); - $html = '
'; + $html = '' + . '' + . '
' + . ''; $document = new DOMDocument('1.0', RCUBE_CHARSET); @$document->loadHTML($html); diff --git a/tests/Framework/Html.php b/tests/Framework/Html.php index edc3466b0..8507477c2 100644 --- a/tests/Framework/Html.php +++ b/tests/Framework/Html.php @@ -117,6 +117,10 @@ class Framework_Html extends PHPUnit_Framework_TestCase 'expression="test == true ? \' test\' : \'\'" ', array('expression' => 'test == true ? \' test\' : \'\''), ), + array( + 'href="http://domain.tld/страница"', + array('href' => 'http://domain.tld/страница'), + ), ); }