From 14c4677eede6263f26b8830917ec6e74409b80c4 Mon Sep 17 00:00:00 2001
From: Aleksander Machniak <alec@alec.pl>
Date: Wed, 15 Aug 2012 05:21:49 -0400
Subject: [PATCH] Fix XSS issue where plain signatures wasn't secured in HTML mode (#1488613)

---
 program/js/app.js |   60 ++++++++++++++++++++++--------------------------------------
 1 files changed, 22 insertions(+), 38 deletions(-)

diff --git a/program/js/app.js b/program/js/app.js
index 2a47fa9..0e6605d 100644
--- a/program/js/app.js
+++ b/program/js/app.js
@@ -60,6 +60,8 @@
     beforeSend: function(xmlhttp){ xmlhttp.setRequestHeader('X-Roundcube-Request', ref.env.request_token); }
   });
 
+  $(window).bind('beforeunload', function() { rcmail.unload = true; });
+
   // set environment variable(s)
   this.set_env = function(p, value)
   {
@@ -766,7 +768,7 @@
 
       case 'always-load':
         if (this.env.uid && this.env.sender) {
-          this.add_contact(urlencode(this.env.sender));
+          this.add_contact(this.env.sender);
           setTimeout(function(){ ref.command('load-images'); }, 300);
           break;
         }
@@ -1622,7 +1624,7 @@
     for (i=0; i<cols.length; i++)
       if (cols[i].id && cols[i].id.match(/^rcm/)) {
         name = cols[i].id.replace(/^rcm/, '');
-        this.env.coltypes.push(name == 'to' ? 'from' : name);
+        this.env.coltypes.push(name);
       }
 
     if ((found = $.inArray('flag', this.env.coltypes)) >= 0)
@@ -1815,7 +1817,7 @@
         html = '<span id="flagicn'+uid+'" class="'+css_class+'">&nbsp;</span>';
       }
       else if (c == 'attachment') {
-        if (/application\/|multipart\/m/.test(flags.ctype))
+        if (/application\/|multipart\/(m|signed)/.test(flags.ctype))
           html = '<span class="attachment">&nbsp;</span>';
         else if (/multipart\/report/.test(flags.ctype))
           html = '<span class="report">&nbsp;</span>';
@@ -1901,7 +1903,7 @@
       // make sure new columns are added at the end of the list
       var i, idx, name, newcols = [], oldcols = this.env.coltypes;
       for (i=0; i<oldcols.length; i++) {
-        name = oldcols[i] == 'to' ? 'from' : oldcols[i];
+        name = oldcols[i];
         idx = $.inArray(name, cols);
         if (idx != -1) {
           newcols.push(name);
@@ -2978,7 +2980,7 @@
       this.set_caret_pos(input_message, this.env.top_posting ? 0 : $(input_message).val().length);
       // add signature according to selected identity
       // if we have HTML editor, signature is added in callback
-      if (input_from.prop('type') == 'select-one' && $("input[name='_draft_saveid']").val() == '') {
+      if (input_from.prop('type') == 'select-one') {
         this.change_identity(input_from[0]);
       }
     }
@@ -3273,8 +3275,7 @@
       input_message = $("[name='_message']"),
       message = input_message.val(),
       is_html = ($("input[name='_is_html']").val() == '1'),
-      sig = this.env.identity,
-      sig_separator = this.env.sig_above && (this.env.compose_mode == 'reply' || this.env.compose_mode == 'forward') ? '---' : '-- ';
+      sig = this.env.identity;
 
     // enable manual signature insert
     if (this.env.signatures && this.env.signatures[id]) {
@@ -3287,12 +3288,8 @@
     if (!is_html) {
       // remove the 'old' signature
       if (show_sig && sig && this.env.signatures && this.env.signatures[sig]) {
-
-        sig = this.env.signatures[sig].is_html ? this.env.signatures[sig].plain_text : this.env.signatures[sig].text;
+        sig = this.env.signatures[sig].text;
         sig = sig.replace(/\r\n/g, '\n');
-
-        if (!sig.match(/^--[ -]\n/m))
-          sig = sig_separator + '\n' + sig;
 
         p = this.env.sig_above ? message.indexOf(sig) : message.lastIndexOf(sig);
         if (p >= 0)
@@ -3300,11 +3297,8 @@
       }
       // add the new signature string
       if (show_sig && this.env.signatures && this.env.signatures[id]) {
-        sig = this.env.signatures[id]['is_html'] ? this.env.signatures[id]['plain_text'] : this.env.signatures[id]['text'];
+        sig = this.env.signatures[id].text;
         sig = sig.replace(/\r\n/g, '\n');
-
-        if (!sig.match(/^--[ -]\n/m))
-          sig = sig_separator + '\n' + sig;
 
         if (this.env.sig_above) {
           if (p >= 0) { // in place of removed signature
@@ -3369,21 +3363,8 @@
         }
       }
 
-      if (this.env.signatures[id]) {
-        if (this.env.signatures[id].is_html) {
-          sig = this.env.signatures[id].text;
-          if (!this.env.signatures[id].plain_text.match(/^--[ -]\r?\n/m))
-            sig = sig_separator + '<br />' + sig;
-        }
-        else {
-          sig = this.env.signatures[id].text;
-          if (!sig.match(/^--[ -]\r?\n/m))
-            sig = sig_separator + '\n' + sig;
-          sig = '<pre>' + sig + '</pre>';
-        }
-
-        sigElem.innerHTML = sig;
-      }
+      if (this.env.signatures[id])
+        sigElem.innerHTML = this.env.signatures[id].html;
     }
 
     this.env.identity = id;
@@ -3527,7 +3508,7 @@
   this.add_contact = function(value)
   {
     if (value)
-      this.http_post('addcontact', '_address='+value);
+      this.http_post('addcontact', {_address: value});
 
     return true;
   };
@@ -3783,7 +3764,7 @@
       return;
 
     // ...new search value contains old one and previous search was not finished or its result was empty
-    if (old_value && old_value.length && q.indexOf(old_value) == 0 && (!ac || !ac.num) && this.env.contacts && !this.env.contacts.length)
+    if (old_value && old_value.length && q.indexOf(old_value) == 0 && (!ac || ac.num <= 0) && this.env.contacts && !this.env.contacts.length)
       return;
 
     var i, lock, source, xhr, reqid = new Date().getTime(),
@@ -3998,8 +3979,7 @@
 
     // if a group is currently selected, and there is at least one contact selected
     // thend we can enable the group-remove-selected command
-    this.enable_command('group-remove-selected', typeof this.env.group != 'undefined' && list.selection.length > 0);
-
+    this.enable_command('group-remove-selected', this.env.group && list.selection.length > 0);
     this.enable_command('compose', this.env.group || list.selection.length > 0);
     this.enable_command('edit', id && writable);
     this.enable_command('delete', list.selection.length && writable);
@@ -5644,7 +5624,7 @@
 
   // for reordering column array (Konqueror workaround)
   // and for setting some message list global variables
-  this.set_message_coltypes = function(coltypes, repl)
+  this.set_message_coltypes = function(coltypes, repl, smart_col)
   {
     var list = this.message_list,
       thead = list ? list.list.tHead : null,
@@ -5672,7 +5652,7 @@
 
       for (n=0, len=this.env.coltypes.length; n<len; n++) {
         col = this.env.coltypes[n];
-        if ((cell = thead.rows[0].cells[n]) && (col=='from' || col=='to')) {
+        if ((cell = thead.rows[0].cells[n]) && (col == 'from' || col == 'to' || col == 'fromto')) {
           cell.id = 'rcm'+col;
           // if we have links for sorting, it's a bit more complicated...
           if (cell.firstChild && cell.firstChild.tagName.toLowerCase()=='a') {
@@ -5680,7 +5660,7 @@
             cell.onclick = function(){ return rcmail.command('sort', this.__col, this); };
             cell.__col = col;
           }
-          cell.innerHTML = this.get_label(col);
+          cell.innerHTML = this.get_label(col == 'fromto' ? smart_col : col);
         }
       }
     }
@@ -6129,6 +6109,10 @@
     this.set_busy(false, null, lock);
     request.abort();
 
+    // don't display error message on page unload (#1488547)
+    if (this.unload)
+      return;
+
     if (request.status && errmsg)
       this.display_message(this.get_label('servererror') + ' (' + errmsg + ')', 'error');
     else if (status == 'timeout')

--
Gitblit v1.9.1