From 376cbfd4f2dfcf455717409b70d9d056cbeb08b1 Mon Sep 17 00:00:00 2001 From: Aleksander Machniak Date: Mon, 15 Dec 2014 13:47:55 +0100 Subject: Fix bugs where CSRF attacks were still possible on some requests --- CHANGELOG | 1 + index.php | 1 + plugins/acl/acl.js | 27 ++++++++++++++-------- plugins/acl/acl.php | 12 +++++----- .../lib/Roundcube/rcube_sieve_engine.php | 10 ++++---- program/js/app.js | 9 ++++++-- program/steps/addressbook/delete.inc | 5 ++-- program/steps/addressbook/func.inc | 4 ++-- 8 files changed, 43 insertions(+), 26 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 8fcb2014e..370ab1a3e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -18,6 +18,7 @@ CHANGELOG Roundcube Webmail - Fix possible issues in skin/skin_path config handling (#1490125) - Fix lack of delimiter for recipient addresses in smtp_log (#1490150) - Fix generation of Blowfish-based password hashes (#1490184) +- Fix bugs where CSRF attacks were still possible on some requests RELEASE 1.0.3 ------------- diff --git a/index.php b/index.php index 082f11ea4..a3f54bb27 100644 --- a/index.php +++ b/index.php @@ -172,6 +172,7 @@ if ($RCMAIL->task == 'login' && $RCMAIL->action == 'login') { // end session (after optional referer check) else if ($RCMAIL->task == 'logout' && isset($_SESSION['user_id']) + && $RCMAIL->check_request(rcube_utils::INPUT_GET) && (!$RCMAIL->config->get('referer_check') || rcube_utils::check_referer()) ) { $userdata = array( diff --git a/plugins/acl/acl.js b/plugins/acl/acl.js index acea60a4c..795a35a6f 100644 --- a/plugins/acl/acl.js +++ b/plugins/acl/acl.js @@ -62,8 +62,11 @@ rcube_webmail.prototype.acl_delete = function() var users = this.acl_get_usernames(); if (users && users.length && confirm(this.get_label('acl.deleteconfirm'))) { - this.http_request('settings/plugin.acl', '_act=delete&_user='+urlencode(users.join(',')) - + '&_mbox='+urlencode(this.env.mailbox), + this.http_post('settings/plugin.acl', { + _act: 'delete', + _user: users.join(','), + _mbox: this.env.mailbox + }, this.set_busy(true, 'acl.deleting')); } } @@ -71,7 +74,7 @@ rcube_webmail.prototype.acl_delete = function() // Save ACL data rcube_webmail.prototype.acl_save = function() { - var user = $('#acluser', this.acl_form).val(), rights = '', type; + var data, type, rights = '', user = $('#acluser', this.acl_form).val(); $((this.env.acl_advanced ? '#advancedrights :checkbox' : '#simplerights :checkbox'), this.acl_form).map(function() { if (this.checked) @@ -92,12 +95,18 @@ rcube_webmail.prototype.acl_save = function() return; } - this.http_request('settings/plugin.acl', '_act=save' - + '&_user='+urlencode(user) - + '&_acl=' +rights - + '&_mbox='+urlencode(this.env.mailbox) - + (this.acl_id ? '&_old='+this.acl_id : ''), - this.set_busy(true, 'acl.saving')); + data = { + _act: 'save', + _user: user, + _acl: rights, + _mbox: this.env.mailbox + } + + if (this.acl_id) { + data._old = this.acl_id; + } + + this.http_post('settings/plugin.acl', data, this.set_busy(true, 'acl.saving')); } // Cancel/Hide form diff --git a/plugins/acl/acl.php b/plugins/acl/acl.php index a8b8f58ad..b440c24a1 100644 --- a/plugins/acl/acl.php +++ b/plugins/acl/acl.php @@ -443,10 +443,10 @@ class acl extends rcube_plugin */ private function action_save() { - $mbox = trim(rcube_utils::get_input_value('_mbox', rcube_utils::INPUT_GPC, true)); // UTF7-IMAP - $user = trim(rcube_utils::get_input_value('_user', rcube_utils::INPUT_GPC)); - $acl = trim(rcube_utils::get_input_value('_acl', rcube_utils::INPUT_GPC)); - $oldid = trim(rcube_utils::get_input_value('_old', rcube_utils::INPUT_GPC)); + $mbox = trim(rcube_utils::get_input_value('_mbox', rcube_utils::INPUT_POST, true)); // UTF7-IMAP + $user = trim(rcube_utils::get_input_value('_user', rcube_utils::INPUT_POST)); + $acl = trim(rcube_utils::get_input_value('_acl', rcube_utils::INPUT_POST)); + $oldid = trim(rcube_utils::get_input_value('_old', rcube_utils::INPUT_POST)); $acl = array_intersect(str_split($acl), $this->rights_supported()); $users = $oldid ? array($user) : explode(',', $user); @@ -499,8 +499,8 @@ class acl extends rcube_plugin */ private function action_delete() { - $mbox = trim(rcube_utils::get_input_value('_mbox', rcube_utils::INPUT_GPC, true)); //UTF7-IMAP - $user = trim(rcube_utils::get_input_value('_user', rcube_utils::INPUT_GPC)); + $mbox = trim(rcube_utils::get_input_value('_mbox', rcube_utils::INPUT_POST, true)); //UTF7-IMAP + $user = trim(rcube_utils::get_input_value('_user', rcube_utils::INPUT_POST)); $user = explode(',', $user); diff --git a/plugins/managesieve/lib/Roundcube/rcube_sieve_engine.php b/plugins/managesieve/lib/Roundcube/rcube_sieve_engine.php index a93e389b9..7d7ea99d3 100644 --- a/plugins/managesieve/lib/Roundcube/rcube_sieve_engine.php +++ b/plugins/managesieve/lib/Roundcube/rcube_sieve_engine.php @@ -310,7 +310,7 @@ class rcube_sieve_engine } } else if ($action == 'setact' && !$error) { - $script_name = rcube_utils::get_input_value('_set', rcube_utils::INPUT_GPC, true); + $script_name = rcube_utils::get_input_value('_set', rcube_utils::INPUT_POST, true); $result = $this->activate_script($script_name); $kep14 = $this->rc->config->get('managesieve_kolab_master'); @@ -324,7 +324,7 @@ class rcube_sieve_engine } } else if ($action == 'deact' && !$error) { - $script_name = rcube_utils::get_input_value('_set', rcube_utils::INPUT_GPC, true); + $script_name = rcube_utils::get_input_value('_set', rcube_utils::INPUT_POST, true); $result = $this->deactivate_script($script_name); if ($result === true) { @@ -337,7 +337,7 @@ class rcube_sieve_engine } } else if ($action == 'setdel' && !$error) { - $script_name = rcube_utils::get_input_value('_set', rcube_utils::INPUT_GPC, true); + $script_name = rcube_utils::get_input_value('_set', rcube_utils::INPUT_POST, true); $result = $this->remove_script($script_name); if ($result === true) { @@ -381,14 +381,14 @@ class rcube_sieve_engine $this->rc->output->command('managesieve_updatelist', 'list', array('list' => $result)); } else if ($action == 'ruleadd') { - $rid = rcube_utils::get_input_value('_rid', rcube_utils::INPUT_GPC); + $rid = rcube_utils::get_input_value('_rid', rcube_utils::INPUT_POST); $id = $this->genid(); $content = $this->rule_div($fid, $id, false); $this->rc->output->command('managesieve_rulefill', $content, $id, $rid); } else if ($action == 'actionadd') { - $aid = rcube_utils::get_input_value('_aid', rcube_utils::INPUT_GPC); + $aid = rcube_utils::get_input_value('_aid', rcube_utils::INPUT_POST); $id = $this->genid(); $content = $this->action_div($fid, $id, false); diff --git a/program/js/app.js b/program/js/app.js index fe9daddc8..7859ecbea 100644 --- a/program/js/app.js +++ b/program/js/app.js @@ -1336,8 +1336,10 @@ function rcube_webmail() var url = this.get_task_url(task); if (task == 'mail') url += '&_mbox=INBOX'; - else if (task == 'logout' && !this.env.server_error) + else if (task == 'logout' && !this.env.server_error) { + url += '&_token=' + this.env.request_token; this.clear_compose_data(); + } this.redirect(url); }; @@ -1347,7 +1349,10 @@ function rcube_webmail() if (!url) url = this.env.comm_path; - return url.replace(/_task=[a-z0-9_-]+/i, '_task='+task); + if (url.match(/[?&]_task=[a-zA-Z0-9_-]+/)) + return url.replace(/_task=[a-zA-Z0-9_-]+/, '_task=' + task); + else + return url.replace(/\?.*$/, '') + '?_task=' + task; }; this.reload = function(delay) diff --git a/program/steps/addressbook/delete.inc b/program/steps/addressbook/delete.inc index f5b8e4eb5..9a23c59bb 100644 --- a/program/steps/addressbook/delete.inc +++ b/program/steps/addressbook/delete.inc @@ -20,10 +20,11 @@ */ // process ajax requests only -if (!$OUTPUT->ajax_call) +if (!$OUTPUT->ajax_call) { return; +} -$cids = rcmail_get_cids(); +$cids = rcmail_get_cids(null, rcube_utils::INPUT_POST); $delcnt = 0; // remove previous deletes diff --git a/program/steps/addressbook/func.inc b/program/steps/addressbook/func.inc index 2989dada2..625e044a4 100644 --- a/program/steps/addressbook/func.inc +++ b/program/steps/addressbook/func.inc @@ -879,13 +879,13 @@ function rcmail_search_update($return = false) * * @return array List of contact IDs per-source */ -function rcmail_get_cids($filter = null) +function rcmail_get_cids($filter = null, $request_type = rcube_utils::INPUT_GPC) { // contact ID (or comma-separated list of IDs) is provided in two // forms. If _source is an empty string then the ID is a string // containing contact ID and source name in form: - - $cid = rcube_utils::get_input_value('_cid', rcube_utils::INPUT_GPC); + $cid = rcube_utils::get_input_value('_cid', $request_type); $source = (string) rcube_utils::get_input_value('_source', rcube_utils::INPUT_GPC); if (is_array($cid)) { -- cgit v1.2.3