From ca01e25772730cab0117bca0e514140e6c5f67d1 Mon Sep 17 00:00:00 2001
From: Aleksander Machniak <alec@alec.pl>
Date: Sat, 05 Jul 2014 06:33:03 -0400
Subject: [PATCH] Fix security issue in delete-response action - allow only ajax request. Unify code for identities and responses deletion.

---
 program/steps/settings/identities.inc |   22 +++++++++++
 CHANGELOG                             |    1 
 /dev/null                             |   55 ---------------------------
 program/steps/settings/responses.inc  |    8 +--
 program/steps/settings/func.inc       |    1 
 program/js/app.js                     |   22 +++++++---
 6 files changed, 42 insertions(+), 67 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 6b3927a..b420833 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -53,6 +53,7 @@
 - Fix list reload after sending message in another window (#1489931)
 - Fix so address format errors are ignored when saving a draft (#1489954)
 - Fix incorrect label translation in return receipt (#1489963)
+- Fix security issue in delete-response action - allow only ajax request
 
 RELEASE 1.0.1
 -------------
diff --git a/program/js/app.js b/program/js/app.js
index 34871f7..b2d936d 100644
--- a/program/js/app.js
+++ b/program/js/app.js
@@ -3722,10 +3722,7 @@
     // submit delete request
     if (key && confirm(this.get_label('deleteresponseconfirm'))) {
       this.http_post('settings/delete-response', { _key: key }, false);
-      return true;
     }
-
-    return false;
   };
 
   // updates spellchecker buttons on state change
@@ -5700,10 +5697,8 @@
       id = this.env.iid ? this.env.iid : selection[0];
 
     // submit request with appended token
-    if (confirm(this.get_label('deleteidentityconfirm')))
-      this.goto_url('delete-identity', { _iid: id, _token: this.env.request_token }, true);
-
-    return true;
+    if (id && confirm(this.get_label('deleteidentityconfirm')))
+      this.http_post('settings/delete-identity', { _iid: id }, true);
   };
 
   this.update_identity_row = function(id, name, add)
@@ -5749,6 +5744,19 @@
     }
   };
 
+  this.remove_identity = function(id)
+  {
+    var frame, list = this.identity_list,
+      rid = this.html_identifier(id);
+
+    if (list && id) {
+      list.remove_row(rid);
+      if (this.env.contentframe && (frame = this.get_frame_window(this.env.contentframe))) {
+        frame.location.href = this.env.blankpage;
+      }
+    }
+  };
+
 
   /*********************************************************/
   /*********        folder manager methods         *********/
diff --git a/program/steps/settings/delete_identity.inc b/program/steps/settings/delete_identity.inc
deleted file mode 100644
index f776204..0000000
--- a/program/steps/settings/delete_identity.inc
+++ /dev/null
@@ -1,55 +0,0 @@
-<?php
-
-/*
- +-----------------------------------------------------------------------+
- | program/steps/settings/delete_identity.inc                            |
- |                                                                       |
- | This file is part of the Roundcube Webmail client                     |
- | Copyright (C) 2005-2013, The Roundcube Dev Team                       |
- |                                                                       |
- | Licensed under the GNU General Public License version 3 or            |
- | any later version with exceptions for skins & plugins.                |
- | See the README file for a full license statement.                     |
- |                                                                       |
- | PURPOSE:                                                              |
- |   Delete the submitted identities (IIDs) from the database            |
- |                                                                       |
- +-----------------------------------------------------------------------+
- | Author: Thomas Bruederli <roundcube@gmail.com>                        |
- +-----------------------------------------------------------------------+
-*/
-
-$iid = rcube_utils::get_input_value('_iid', rcube_utils::INPUT_GPC);
-
-// check request token
-if (!$OUTPUT->ajax_call && !$RCMAIL->check_request(rcube_utils::INPUT_GPC)) {
-    $OUTPUT->show_message('invalidrequest', 'error');
-    $RCMAIL->overwrite_action('identities');
-    return;
-}
-
-if ($iid && preg_match('/^[0-9]+(,[0-9]+)*$/', $iid)) {
-    $plugin = $RCMAIL->plugins->exec_hook('identity_delete', array('id' => $iid));
-
-    $deleted = !$plugin['abort'] ? $RCMAIL->user->delete_identity($iid) : $plugin['result'];
-
-    if ($deleted > 0 && $deleted !== false) {
-        $OUTPUT->show_message('deletedsuccessfully', 'confirmation', null, false);
-    }
-    else {
-        $msg = $plugin['message'] ? $plugin['message'] : ($deleted < 0 ? 'nodeletelastidentity' : 'errorsaving');
-        $OUTPUT->show_message($msg, 'error', null, false);
-    }
-
-    // send response
-    if ($OUTPUT->ajax_call) {
-        $OUTPUT->send();
-    }
-}
-
-if ($OUTPUT->ajax_call) {
-    exit;
-}
-
-// go to identities page
-$RCMAIL->overwrite_action('identities');
diff --git a/program/steps/settings/func.inc b/program/steps/settings/func.inc
index 40b70b1..8a96ada 100644
--- a/program/steps/settings/func.inc
+++ b/program/steps/settings/func.inc
@@ -44,6 +44,7 @@
     'add-response'  => 'edit_response.inc',
     'save-response' => 'edit_response.inc',
     'delete-response' => 'responses.inc',
+    'delete-identity' => 'identities.inc',
     'upload-display'  => 'upload.inc',
 ));
 
diff --git a/program/steps/settings/identities.inc b/program/steps/settings/identities.inc
index e19c16c..f43edc1 100644
--- a/program/steps/settings/identities.inc
+++ b/program/steps/settings/identities.inc
@@ -19,6 +19,28 @@
  +-----------------------------------------------------------------------+
 */
 
+if ($RCMAIL->action == 'delete-identity' && $OUTPUT->ajax_call) {
+    $iid = rcube_utils::get_input_value('_iid', rcube_utils::INPUT_POST);
+
+    if ($iid && preg_match('/^[0-9]+(,[0-9]+)*$/', $iid)) {
+        $plugin = $RCMAIL->plugins->exec_hook('identity_delete', array('id' => $iid));
+
+        $deleted = !$plugin['abort'] ? $RCMAIL->user->delete_identity($iid) : $plugin['result'];
+
+        if ($deleted > 0 && $deleted !== false) {
+            $OUTPUT->show_message('deletedsuccessfully', 'confirmation', null, false);
+            $OUTPUT->command('remove_identity', $iid);
+        }
+        else {
+            $msg = $plugin['message'] ? $plugin['message'] : ($deleted < 0 ? 'nodeletelastidentity' : 'errorsaving');
+            $OUTPUT->show_message($msg, 'error', null, false);
+        }
+    }
+
+    $OUTPUT->send();
+}
+
+
 define('IDENTITIES_LEVEL', intval($RCMAIL->config->get('identities_level', 0)));
 
 $OUTPUT->set_pagetitle($RCMAIL->gettext('identities'));
diff --git a/program/steps/settings/responses.inc b/program/steps/settings/responses.inc
index ddd1924..117e17f 100644
--- a/program/steps/settings/responses.inc
+++ b/program/steps/settings/responses.inc
@@ -51,8 +51,8 @@
     $RCMAIL->output->send();
 }
 
-if ($RCMAIL->action == 'delete-response') {
-    if ($key = rcube_utils::get_input_value('_key', rcube_utils::INPUT_GPC)) {
+if ($RCMAIL->action == 'delete-response' && $RCMAIL->output->ajax_call) {
+    if ($key = rcube_utils::get_input_value('_key', rcube_utils::INPUT_POST)) {
         $responses = $RCMAIL->get_compose_responses(false, true);
         foreach ($responses as $i => $response) {
             if (empty($response['key']))
@@ -70,9 +70,7 @@
         $RCMAIL->output->command('remove_response', $key);
     }
 
-    if ($RCMAIL->output->ajax_call) {
-        $RCMAIL->output->send();
-    }
+    $RCMAIL->output->send();
 }
 
 

--
Gitblit v1.9.1