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/steps/mail/compose.inc |   53 ++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/program/steps/mail/compose.inc b/program/steps/mail/compose.inc
index 8152f5d..2994bf0 100644
--- a/program/steps/mail/compose.inc
+++ b/program/steps/mail/compose.inc
@@ -532,7 +532,7 @@
 
 function rcmail_compose_header_from($attrib)
 {
-  global $MESSAGE, $OUTPUT;
+  global $MESSAGE, $OUTPUT, $RCMAIL, $compose_mode;
 
   // pass the following attributes to the form class
   $field_attrib = array('name' => '_from');
@@ -543,6 +543,8 @@
   if (count($MESSAGE->identities))
   {
     $a_signatures = array();
+    $separator    = $RCMAIL->config->get('sig_above')
+      && ($compose_mode == RCUBE_COMPOSE_REPLY || $compose_mode == RCUBE_COMPOSE_FORWARD) ? '---' : '-- ';
 
     $field_attrib['onchange'] = JS_OBJECT_NAME.".change_identity(this)";
     $select_from = new html_select($field_attrib);
@@ -556,13 +558,27 @@
       // add signature to array
       if (!empty($sql_arr['signature']) && empty($COMPOSE['param']['nosig']))
       {
-        $a_signatures[$identity_id]['text'] = $sql_arr['signature'];
-        $a_signatures[$identity_id]['is_html'] = ($sql_arr['html_signature'] == 1) ? true : false;
-        if ($a_signatures[$identity_id]['is_html'])
-        {
-            $h2t = new html2text($a_signatures[$identity_id]['text'], false, false);
-            $a_signatures[$identity_id]['plain_text'] = trim($h2t->get_text());
+        $text = $html = $sql_arr['signature'];
+
+        if ($sql_arr['html_signature']) {
+            $h2t  = new html2text($sql_arr['signature'], false, false);
+            $text = trim($h2t->get_text());
         }
+        else {
+            $html = htmlentities($html, ENT_NOQUOTES, RCMAIL_CHARSET);
+        }
+
+        if (!preg_match('/^--[ -]\r?\n/m', $text)) {
+            $text = $separator . "\n" . $text;
+            $html = $separator . "<br>" . $html;
+        }
+
+        if (!$sql_arr['html_signature']) {
+            $html = "<pre>" . $html . "</pre>";
+        }
+
+        $a_signatures[$identity_id]['text'] = $text;
+        $a_signatures[$identity_id]['html'] = $html;
       }
     }
 
@@ -632,7 +648,8 @@
 
     if (!empty($MESSAGE->parts)) {
       foreach ($MESSAGE->parts as $part) {
-        if ($part->type != 'content' || !$part->size) {
+        // skip no-content and attachment parts (#1488557)
+        if ($part->type != 'content' || !$part->size || $MESSAGE->is_attachment($part)) {
           continue;
         }
 
@@ -668,9 +685,9 @@
   if ($isHtml && preg_match('#<img src="\./program/blocked\.gif"#', $body)) {
     if ($attachment = rcmail_save_image('program/blocked.gif', 'image/gif')) {
       $COMPOSE['attachments'][$attachment['id']] = $attachment;
-      $body = preg_replace('#\./program/blocked\.gif#',
-        $RCMAIL->comm_path.'&_action=display-attachment&_file=rcmfile'.$attachment['id'].'&_id='.$COMPOSE['id'],
-        $body);
+      $url = sprintf('%s&_id=%s&_action=display-attachment&_file=rcmfile%s',
+        $RCMAIL->comm_path, $COMPOSE['id'], $attachment['id']);
+      $body = preg_replace('#\./program/blocked\.gif#', $url, $body);
     }
   }
 
@@ -960,18 +977,18 @@
         "<tr><th align=\"right\" nowrap=\"nowrap\" valign=\"baseline\">%s: </th><td>%s</td></tr>",
       rcube_label('subject'), Q($MESSAGE->subject),
       rcube_label('date'), Q($date),
-      rcube_label('from'), htmlspecialchars(Q($MESSAGE->get_header('from'), 'replace'), ENT_COMPAT, $charset),
-      rcube_label('to'), htmlspecialchars(Q($MESSAGE->get_header('to'), 'replace'), ENT_COMPAT, $charset));
+      rcube_label('from'), Q($MESSAGE->get_header('from'), 'replace'),
+      rcube_label('to'), Q($MESSAGE->get_header('to'), 'replace'));
 
     if ($MESSAGE->headers->cc)
       $prefix .= sprintf("<tr><th align=\"right\" nowrap=\"nowrap\" valign=\"baseline\">%s: </th><td>%s</td></tr>",
         rcube_label('cc'),
-        htmlspecialchars(Q($MESSAGE->get_header('cc'), 'replace'), ENT_COMPAT, $charset));
+        Q($MESSAGE->get_header('cc'), 'replace'));
 
     if ($MESSAGE->headers->replyto && $MESSAGE->headers->replyto != $MESSAGE->headers->from)
       $prefix .= sprintf("<tr><th align=\"right\" nowrap=\"nowrap\" valign=\"baseline\">%s: </th><td>%s</td></tr>",
         rcube_label('replyto'),
-        htmlspecialchars(Q($MESSAGE->get_header('replyto'), 'replace'), ENT_COMPAT, $charset));
+        Q($MESSAGE->get_header('replyto'), 'replace'));
 
     $prefix .= "</tbody></table><br>";
   }
@@ -1051,7 +1068,8 @@
       if (!$skip && ($attachment = rcmail_save_attachment($message, $pid))) {
         $COMPOSE['attachments'][$attachment['id']] = $attachment;
         if ($bodyIsHtml && ($part->content_id || $part->content_location)) {
-          $url = $RCMAIL->comm_path.'&_action=display-attachment&_file=rcmfile'.$attachment['id'].'&_id='.$COMPOSE['id'];
+          $url = sprintf('%s&_id=%s&_action=display-attachment&_file=rcmfile%s',
+            $RCMAIL->comm_path, $COMPOSE['id'], $attachment['id']);
           if ($part->content_id)
             $cid_map['cid:'.$part->content_id] = $url;
           else
@@ -1076,7 +1094,8 @@
     if (($part->content_id || $part->content_location) && $part->filename) {
       if ($attachment = rcmail_save_attachment($message, $pid)) {
         $COMPOSE['attachments'][$attachment['id']] = $attachment;
-        $url = $RCMAIL->comm_path.'&_action=display-attachment&_file=rcmfile'.$attachment['id'].'&_id='.$COMPOSE['id'];
+          $url = sprintf('%s&_id=%s&_action=display-attachment&_file=rcmfile%s',
+            $RCMAIL->comm_path, $COMPOSE['id'], $attachment['id']);
         if ($part->content_id)
           $cid_map['cid:'.$part->content_id] = $url;
         else

--
Gitblit v1.9.1