From 48ba4414b33c8982f8232b06f06d68f3213aa986 Mon Sep 17 00:00:00 2001 From: Aleksander Machniak Date: Wed, 22 Oct 2014 14:29:44 +0200 Subject: Fix download of attachments that are part of TNEF message (#1490091) Rcube_message_part::body content should never be modified by code out of the rcube_message. Added convenient rcube_message::get_part_body() method, making rcube_message::get_part_content() deprecated. --- CHANGELOG | 1 + plugins/enigma/lib/enigma_driver_phpssl.php | 2 +- plugins/vcard_attachments/vcard_attachments.php | 2 +- plugins/zipdownload/zipdownload.php | 20 +-- program/lib/Roundcube/rcube_message.php | 220 ++++++++++++++++++------ program/steps/mail/compose.inc | 16 +- program/steps/mail/func.inc | 32 ++-- program/steps/mail/get.inc | 40 ++--- tests/MailFunc.php | 16 +- 9 files changed, 226 insertions(+), 123 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 219071c67..9b353cf1a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -53,6 +53,7 @@ CHANGELOG Roundcube Webmail - Fix regression in SHAA password generation in ldap driver of password plugin (#1490094) - Fix displaying of HTML messages with absolutely positioned elements in Larry skin (#1490103) - Fix font style display issue in HTML messages with styled elements (#1490101) +- Fix download of attachments that are part of TNEF message (#1490091) RELEASE 1.0.3 ------------- diff --git a/plugins/enigma/lib/enigma_driver_phpssl.php b/plugins/enigma/lib/enigma_driver_phpssl.php index 50af44762..fcd15db73 100644 --- a/plugins/enigma/lib/enigma_driver_phpssl.php +++ b/plugins/enigma/lib/enigma_driver_phpssl.php @@ -95,7 +95,7 @@ class enigma_driver_phpssl extends enigma_driver $fh = fopen($msg_file, "w"); if ($struct->mime_id) { - $message->get_part_content($struct->mime_id, $fh, true, 0, false); + $message->get_part_body($struct->mime_id, false, 0, $fh); } else { $this->rc->storage->get_raw_body($message->uid, $fh); diff --git a/plugins/vcard_attachments/vcard_attachments.php b/plugins/vcard_attachments/vcard_attachments.php index cf7e22d3a..74718be6f 100644 --- a/plugins/vcard_attachments/vcard_attachments.php +++ b/plugins/vcard_attachments/vcard_attachments.php @@ -65,7 +65,7 @@ class vcard_attachments extends rcube_plugin $attach_script = false; foreach ($this->vcard_parts as $part) { - $vcards = rcube_vcard::import($this->message->get_part_content($part, null, true)); + $vcards = rcube_vcard::import($this->message->get_part_body($part, true)); // successfully parsed vcards? if (empty($vcards)) { diff --git a/plugins/zipdownload/zipdownload.php b/plugins/zipdownload/zipdownload.php index edb8188cc..2e103ceb0 100644 --- a/plugins/zipdownload/zipdownload.php +++ b/plugins/zipdownload/zipdownload.php @@ -144,20 +144,14 @@ class zipdownload extends rcube_plugin } } - $disp_name = $this->_convert_filename($filename); + $disp_name = $this->_convert_filename($filename); + $tmpfn = tempnam($temp_dir, 'zipattach'); + $tmpfp = fopen($tmpfn, 'w'); + $tempfiles[] = $tmpfn; - if ($part->body) { - $orig_message_raw = $part->body; - $zip->addFromString($disp_name, $orig_message_raw); - } - else { - $tmpfn = tempnam($temp_dir, 'zipattach'); - $tmpfp = fopen($tmpfn, 'w'); - $imap->get_message_part($message->uid, $part->mime_id, $part, null, $tmpfp, true); - $tempfiles[] = $tmpfn; - fclose($tmpfp); - $zip->addFile($tmpfn, $disp_name); - } + $message->get_part_body($part->mime_id, false, 0, $tmpfp); + $zip->addFile($tmpfn, $disp_name); + fclose($tmpfp); } $zip->close(); diff --git a/program/lib/Roundcube/rcube_message.php b/program/lib/Roundcube/rcube_message.php index a648ae722..a00f6bfa5 100644 --- a/program/lib/Roundcube/rcube_message.php +++ b/program/lib/Roundcube/rcube_message.php @@ -3,7 +3,7 @@ /* +-----------------------------------------------------------------------+ | This file is part of the Roundcube Webmail client | - | Copyright (C) 2008-2010, The Roundcube Dev Team | + | Copyright (C) 2008-2014, The Roundcube Dev Team | | | | Licensed under the GNU General Public License version 3 or | | any later version with exceptions for skins & plugins. | @@ -61,6 +61,8 @@ class rcube_message public $sender = null; public $is_safe = false; + const BODY_MAX_SIZE = 1048576; // 1MB + /** * __construct @@ -176,6 +178,7 @@ class rcube_message * @param boolean $formatted Enables formatting of text/* parts bodies * * @return string Part content + * @deprecated */ public function get_part_content($mime_id, $fp = null, $skip_charset_conv = false, $max_bytes = 0, $formatted = true) { @@ -197,6 +200,125 @@ class rcube_message } + /** + * Get content of a specific part of this message + * + * @param string $mime_id Part ID + * @param boolean $formatted Enables formatting of text/* parts bodies + * @param int $max_bytes Only return/read this number of bytes + * @param mixed $mode NULL to return a string, -1 to print body + * or file pointer to save the body into + * + * @return string|bool Part content or operation status + */ + public function get_part_body($mime_id, $formatted = false, $max_bytes = 0, $mode = null) + { + if (!($part = $this->mime_parts[$mime_id])) { + return; + } + + // only text parts can be formatted + $formatted = $formatted && $part->ctype_primary == 'text'; + + // part body not fetched yet... save in memory if it's small enough + if ($part->body === null && is_numeric($mime_id) && $part->size < self::BODY_MAX_SIZE) { + // Warning: body here should be always unformatted + $part->body = $this->storage->get_message_part($this->uid, $mime_id, $part, + null, null, true, 0, false); + } + + // body stored in message structure (winmail/inline-uuencode) + if ($part->body !== null || $part->encoding == 'stream') { + $body = $part->body; + + if ($formatted && $body) { + $body = self::format_part_body($body, $part, $this->headers->charset); + } + + if ($max_bytes && strlen($body) > $max_bytes) { + $body = substr($body, 0, $max_bytes); + } + + if (is_resource($mode)) { + if ($body !== false) { + fwrite($mode, $body); + rewind($mode); + } + + return $body !== false; + } + + if ($mode === -1) { + if ($body !== false) { + print($body); + } + + return $body !== false; + } + + return $body; + } + + // get the body from IMAP + $this->storage->set_folder($this->folder); + + $body = $this->storage->get_message_part($this->uid, $mime_id, $part, + $mode === -1, is_resource($mode) ? $mode : null, !$formatted, $max_bytes, $formatted); + + if (!$mode && $body && $formatted) { + $body = self::format_part_body($body, $part, $this->headers->charset); + } + + if (is_resource($mode)) { + rewind($mode); + return $body !== false; + } + + return $body; + } + + + /** + * Format text message part for display + * + * @param string $body Part body + * @param rcube_message_part $part Part object + * @param string $default_charset Fallback charset if part charset is not specified + * + * @return string Formatted body + */ + public static function format_part_body($body, $part, $default_charset = null) + { + // remove useless characters + $body = preg_replace('/[\t\r\0\x0B]+\n/', "\n", $body); + + // remove NULL characters if any (#1486189) + if (strpos($body, "\x00") !== false) { + $body = str_replace("\x00", '', $body); + } + + // detect charset... + if (!$part->charset || strtoupper($part->charset) == 'US-ASCII') { + // try to extract charset information from HTML meta tag (#1488125) + if ($part->ctype_secondary == 'html' && preg_match('/]+charset=([a-z0-9-_]+)/i', $body, $m)) { + $part->charset = strtoupper($m[1]); + } + else if ($default_charset) { + $part->charset = $default_charset; + } + else { + $rcube = rcube::get_instance(); + $part->charset = $rcube->config->get('default_charset', RCUBE_CHARSET); + } + } + + // ..convert charset encoding + $body = rcube_charset::convert($body, $part->charset); + + return $body; + } + + /** * Determine if the message contains a HTML part. This must to be * a real part not an attachment (or its part) @@ -293,7 +415,7 @@ class rcube_message // check all message parts foreach ($this->mime_parts as $pid => $part) { if ($part->mimetype == 'text/html') { - return $this->get_part_content($pid); + return $this->get_part_body($pid, true); } } } @@ -314,10 +436,10 @@ class rcube_message // check all message parts foreach ($this->mime_parts as $mime_id => $part) { if ($part->mimetype == 'text/plain') { - return $this->get_part_content($mime_id); + return $this->get_part_body($mime_id, true); } else if ($part->mimetype == 'text/html') { - $out = $this->get_part_content($mime_id); + $out = $this->get_part_body($mime_id, true); // create instance of html2text class $txt = new rcube_html2text($out); @@ -371,7 +493,7 @@ class rcube_message // parse headers from message/rfc822 part if (!isset($structure->headers['subject']) && !isset($structure->headers['from'])) { - list($headers, ) = explode("\r\n\r\n", $this->get_part_content($structure->mime_id, null, true, 32768)); + list($headers, ) = explode("\r\n\r\n", $this->get_part_body($structure->mime_id, false, 32768)); $structure->headers = rcube_mime::parse_headers($headers); } } @@ -725,15 +847,13 @@ class rcube_message */ function tnef_decode(&$part) { - // @TODO: attachment may be huge, hadle it via file - if (!isset($part->body)) { - $this->storage->set_folder($this->folder); - $part->body = $this->storage->get_message_part($this->uid, $part->mime_id, $part); - } + // @TODO: attachment may be huge, handle body via file + $body = $this->get_part_body($part->mime_id); + $tnef = new tnef_decoder; + $tnef_arr = $tnef->decompress($body); + $parts = array(); - $parts = array(); - $tnef = new tnef_decoder; - $tnef_arr = $tnef->decompress($part->body); + unset($body); foreach ($tnef_arr as $pid => $winatt) { $tpart = new rcube_message_part; @@ -763,50 +883,54 @@ class rcube_message */ function uu_decode(&$part) { - // @TODO: messages may be huge, hadle body via file - if (!isset($part->body)) { - $this->storage->set_folder($this->folder); - $part->body = $this->storage->get_message_part($this->uid, $part->mime_id, $part); - } + // @TODO: messages may be huge, handle body via file + $part->body = $this->get_part_body($part->mime_id); + $parts = array(); + $pid = 0; - $parts = array(); // FIXME: line length is max.65? - $uu_regexp = '/begin [0-7]{3,4} ([^\n]+)\n/s'; + $uu_regexp_begin = '/begin [0-7]{3,4} ([^\r\n]+)\r?\n/s'; + $uu_regexp_end = '/`\r?\nend((\r?\n)|($))/s'; - if (preg_match_all($uu_regexp, $part->body, $matches, PREG_SET_ORDER)) { - // update message content-type - $part->ctype_primary = 'multipart'; - $part->ctype_secondary = 'mixed'; - $part->mimetype = $part->ctype_primary . '/' . $part->ctype_secondary; - $uu_endstring = "`\nend\n"; - - // add attachments to the structure - foreach ($matches as $pid => $att) { - $startpos = strpos($part->body, $att[1]) + strlen($att[1]) + 1; // "\n" - $endpos = strpos($part->body, $uu_endstring); - $filebody = substr($part->body, $startpos, $endpos-$startpos); + while (preg_match($uu_regexp_begin, $part->body, $matches, PREG_OFFSET_CAPTURE)) { + $startpos = $matches[0][1]; - // remove attachments bodies from the message body - $part->body = substr_replace($part->body, "", $startpos, $endpos+strlen($uu_endstring)-$startpos); + if (!preg_match($uu_regexp_end, $part->body, $m, PREG_OFFSET_CAPTURE, $startpos)) { + break; + } - $uupart = new rcube_message_part; + // update message content-type + if ($part->mimetype != 'multipart/mixed') { + $part->ctype_primary = 'multipart'; + $part->ctype_secondary = 'mixed'; + $part->mimetype = $part->ctype_primary . '/' . $part->ctype_secondary; + } - $uupart->filename = trim($att[1]); - $uupart->encoding = 'stream'; - $uupart->body = convert_uudecode($filebody); - $uupart->size = strlen($uupart->body); - $uupart->mime_id = 'uu.' . $part->mime_id . '.' . $pid; + $endpos = $m[0][1]; + $begin_len = strlen($matches[0][0]); + $end_len = strlen($m[0][0]); - $ctype = rcube_mime::file_content_type($uupart->body, $uupart->filename, 'application/octet-stream', true); - $uupart->mimetype = $ctype; - list($uupart->ctype_primary, $uupart->ctype_secondary) = explode('/', $ctype); + // extract attachment body + $filebody = substr($part->body, $startpos + $begin_len, $endpos - $startpos - $begin_len - 1); + $filebody = str_replace("\r\n", "\n", $filebody); - $parts[] = $uupart; - unset($matches[$pid]); - } + // remove attachment body from the message body + $part->body = substr_replace($part->body, '', $startpos, $endpos + $end_len - $startpos); - // remove attachments bodies from the message body - $part->body = preg_replace($uu_regexp, '', $part->body); + // add attachments to the structure + $uupart = new rcube_message_part; + $uupart->filename = trim($matches[1][0]); + $uupart->encoding = 'stream'; + $uupart->body = convert_uudecode($filebody); + $uupart->size = strlen($uupart->body); + $uupart->mime_id = 'uu.' . $part->mime_id . '.' . $pid; + + $ctype = rcube_mime::file_content_type($uupart->body, $uupart->filename, 'application/octet-stream', true); + $uupart->mimetype = $ctype; + list($uupart->ctype_primary, $uupart->ctype_secondary) = explode('/', $ctype); + + $parts[] = $uupart; + $pid++; } return $parts; diff --git a/program/steps/mail/compose.inc b/program/steps/mail/compose.inc index 1770a1bcb..2b1ca4de6 100644 --- a/program/steps/mail/compose.inc +++ b/program/steps/mail/compose.inc @@ -802,22 +802,14 @@ function rcmail_compose_part_body($part, $isHtml = false) return ''; } - if (empty($part->ctype_parameters) || empty($part->ctype_parameters['charset'])) { - $part->ctype_parameters['charset'] = $MESSAGE->headers->charset; - } - // fetch part if not available - if (!isset($part->body)) { - $part->body = $MESSAGE->get_part_content($part->mime_id); - } + $body = $MESSAGE->get_part_body($part->mime_id, true); // message is cached but not exists (#1485443), or other error - if ($part->body === false) { + if ($body === false) { return ''; } - $body = $part->body; - if ($isHtml) { if ($part->ctype_secondary == 'html') { } @@ -1363,7 +1355,7 @@ function rcmail_save_attachment(&$message, $pid) $path = tempnam($temp_dir, 'rcmAttmnt'); if ($fp = fopen($path, 'w')) { - $message->get_part_content($pid, $fp, true, 0, false); + $message->get_part_body($pid, false, 0, $fp); fclose($fp); } else { @@ -1371,7 +1363,7 @@ function rcmail_save_attachment(&$message, $pid) } } else { - $data = $message->get_part_content($pid, null, true, 0, false); + $data = $message->get_part_body($pid); } $mimetype = $part->ctype_primary . '/' . $part->ctype_secondary; diff --git a/program/steps/mail/func.inc b/program/steps/mail/func.inc index cbeeb05fb..8dced9b18 100644 --- a/program/steps/mail/func.inc +++ b/program/steps/mail/func.inc @@ -864,17 +864,19 @@ function rcmail_wash_html($html, $p, $cid_replaces) * Convert the given message part to proper HTML * which can be displayed the message view * - * @param object rcube_message_part Message part - * @param array Display parameters array + * @param string Message part body + * @param rcube_message_part Message part + * @param array Display parameters array + * * @return string Formatted HTML string */ -function rcmail_print_body($part, $p = array()) +function rcmail_print_body($body, $part, $p = array()) { global $RCMAIL; // trigger plugin hook $data = $RCMAIL->plugins->exec_hook('message_part_before', - array('type' => $part->ctype_secondary, 'body' => $part->body, 'id' => $part->mime_id) + array('type' => $part->ctype_secondary, 'body' => $body, 'id' => $part->mime_id) + $p + array('safe' => false, 'plain' => false, 'inline_html' => true)); // convert html to text/plain @@ -900,7 +902,7 @@ function rcmail_print_body($part, $p = array()) } else { // assert plaintext - $body = $part->body; + $body = $data['body']; $part->ctype_secondary = $data['type'] = 'plain'; } @@ -1072,8 +1074,10 @@ function rcmail_message_headers($attrib, $headers=null) } else if ($hkey == 'subject' && empty($value)) $header_value = $RCMAIL->gettext('nosubject'); - else + else { + $value = is_array($value) ? implode(' ', $value) : $value; $header_value = trim(rcube_mime::decode_header($value, $headers['charset'])); + } $output_headers[$hkey] = array( 'title' => $header_title, @@ -1204,18 +1208,12 @@ function rcmail_message_body($attrib) continue; } - if (empty($part->ctype_parameters) || empty($part->ctype_parameters['charset'])) { - $part->ctype_parameters['charset'] = $MESSAGE->headers->charset; - } - - // fetch part if not available - if (!isset($part->body)) { - $part->body = $MESSAGE->get_part_content($part->mime_id); - } + // fetch part body + $body = $MESSAGE->get_part_body($part->mime_id, true); // extract headers from message/rfc822 parts if ($part->mimetype == 'message/rfc822') { - $msgpart = rcube_mime::parse_message($part->body); + $msgpart = rcube_mime::parse_message($body); if (!empty($msgpart->headers)) { $part = $msgpart; $out .= html::div('message-partheaders', rcmail_message_headers(sizeof($header_attrib) ? $header_attrib : null, $part->headers)); @@ -1223,14 +1221,14 @@ function rcmail_message_body($attrib) } // message is cached but not exists (#1485443), or other error - if ($part->body === false) { + if ($body === false) { rcmail_message_error($MESSAGE->uid); } $plugin = $RCMAIL->plugins->exec_hook('message_body_prefix', array('part' => $part, 'prefix' => '')); - $body = rcmail_print_body($part, array('safe' => $safe_mode, 'plain' => !$RCMAIL->config->get('prefer_html'))); + $body = rcmail_print_body($body, $part, array('safe' => $safe_mode, 'plain' => !$RCMAIL->config->get('prefer_html'))); if ($part->ctype_secondary == 'html') { $body = rcmail_html4inline($body, $attrib['id'], 'rcmBody', $attrs, $safe_mode); diff --git a/program/steps/mail/get.inc b/program/steps/mail/get.inc index b260d2c85..948465604 100644 --- a/program/steps/mail/get.inc +++ b/program/steps/mail/get.inc @@ -130,7 +130,7 @@ else if (strlen($part_id)) { $extensions = rcube_mime::get_mime_extensions($mimetype); if ($plugin['body']) { - $part->body = $plugin['body']; + $body = $plugin['body']; } // compare file mimetype with the stated content-type headers and file extension to avoid malicious operations @@ -142,15 +142,10 @@ else if (strlen($part_id)) { // 2. detect the real mimetype of the attachment part and compare it with the stated mimetype and filename extension if ($valid || !$file_extension || $mimetype == 'application/octet-stream' || stripos($mimetype, 'text/') === 0) { - if ($part->body) // part body is already loaded - $body = $part->body; - else if ($part->size && $part->size < 1024*1024) // load the entire part if it's small enough - $body = $part->body = $MESSAGE->get_part_content($part->mime_id); - else // fetch the first 2K of the message part - $body = $MESSAGE->get_part_content($part->mime_id, null, true, 2048); + $tmp_body = $body ?: $MESSAGE->get_part_body($part->mime_id, false, 2048); // detect message part mimetype - $real_mimetype = rcube_mime::file_content_type($body, $part->filename, $mimetype, true, true); + $real_mimetype = rcube_mime::file_content_type($tmp_body, $part->filename, $mimetype, true, true); list($real_ctype_primary, $real_ctype_secondary) = explode('/', $real_mimetype); // accept text/plain with any extension @@ -251,15 +246,15 @@ else if (strlen($part_id)) { } else { // get part body if not available - if (!$part->body) { - $part->body = $MESSAGE->get_part_content($part->mime_id); + if (!isset($body)) { + $body = $MESSAGE->get_part_body($part->mime_id, true); } // show images? rcmail_check_safe($MESSAGE); // render HTML body - $out = rcmail_print_body($part, array('safe' => $MESSAGE->is_safe, 'inline_html' => false)); + $out = rcmail_print_body($body, $part, array('safe' => $MESSAGE->is_safe, 'inline_html' => false)); // insert remote objects warning into HTML body if ($REMOTE_OBJECTS) { @@ -280,7 +275,7 @@ else if (strlen($part_id)) { } // check connection status - if ($part->size && empty($part->body)) { + if ($part->size && empty($body)) { check_storage_status(); } @@ -320,12 +315,12 @@ else if (strlen($part_id)) { $file_path = tempnam($temp_dir, 'rcmAttmnt'); // write content to temp file - if ($part->body) { - $saved = file_put_contents($file_path, $part->body); + if ($body) { + $saved = file_put_contents($file_path, $body); } else if ($part->size) { $fd = fopen($file_path, 'w'); - $saved = $RCMAIL->storage->get_message_part($MESSAGE->uid, $part->mime_id, $part, false, $fd); + $saved = $MESSAGE->get_part_body($part->mime_id, false, 0, $fd); fclose($fd); } @@ -341,22 +336,22 @@ else if (strlen($part_id)) { } // do content filtering to avoid XSS through fake images else if (!empty($_REQUEST['_embed']) && $browser->ie && $browser->ver <= 8) { - if ($part->body) { - echo preg_match('/<(script|iframe|object)/i', $part->body) ? '' : $part->body; + if ($body) { + echo preg_match('/<(script|iframe|object)/i', $body) ? '' : $body; $sent = true; } else if ($part->size) { $stdout = fopen('php://output', 'w'); stream_filter_register('rcube_content', 'rcube_content_filter') or die('Failed to register content filter'); stream_filter_append($stdout, 'rcube_content'); - $sent = $RCMAIL->storage->get_message_part($MESSAGE->uid, $part->mime_id, $part, false, $stdout); + $sent = $MESSAGE->get_part_body($part->mime_id, true, 0, $stdout); } } // send part as-it-is else { - if ($part->body && empty($plugin['download'])) { - header("Content-Length: " . strlen($part->body)); - echo $part->body; + if ($body && empty($plugin['download'])) { + header("Content-Length: " . strlen($body)); + echo $body; $sent = true; } else if ($part->size) { @@ -364,8 +359,7 @@ else if (strlen($part_id)) { header("Content-Length: $size"); } - // 8th argument disables re-formatting of text/* parts (#1489267) - $sent = $RCMAIL->storage->get_message_part($MESSAGE->uid, $part->mime_id, $part, true, null, false, 0, false); + $sent = $MESSAGE->get_part_body($part->mime_id, false, 0, -1); } } diff --git a/tests/MailFunc.php b/tests/MailFunc.php index 05f26324e..7fb78ef9e 100644 --- a/tests/MailFunc.php +++ b/tests/MailFunc.php @@ -42,7 +42,7 @@ class MailFunc extends PHPUnit_Framework_TestCase $part->replaces = array('ex1.jpg' => 'part_1.2.jpg', 'ex2.jpg' => 'part_1.2.jpg'); // render HTML in normal mode - $html = rcmail_html4inline(rcmail_print_body($part, array('safe' => false)), 'foo'); + $html = rcmail_html4inline(rcmail_print_body($part->body, $part, array('safe' => false)), 'foo'); $this->assertRegExp('/src="'.$part->replaces['ex1.jpg'].'"/', $html, "Replace reference to inline image"); $this->assertRegExp('#background="./program/resources/blocked.gif"#', $html, "Replace external background image"); @@ -56,7 +56,7 @@ class MailFunc extends PHPUnit_Framework_TestCase $this->assertTrue($GLOBALS['REMOTE_OBJECTS'], "Remote object detected"); // render HTML in safe mode - $html2 = rcmail_html4inline(rcmail_print_body($part, array('safe' => true)), 'foo'); + $html2 = rcmail_html4inline(rcmail_print_body($part->body, $part, array('safe' => true)), 'foo'); $this->assertRegExp('/