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