From 11526305f506245af55e8ae7ea31faec49dfd98d Mon Sep 17 00:00:00 2001 From: thomascube Date: Mon, 2 Mar 2009 17:34:18 +0000 Subject: Revert r2322; this is done in rcmail_html4inline() and now secured + fix tests --- program/include/rcmail.php | 2 +- program/steps/mail/func.inc | 28 ++++++++++++++-------------- tests/mailfunc.php | 13 +++++++++---- tests/src/htmlxss.txt | 2 +- 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/program/include/rcmail.php b/program/include/rcmail.php index 51ca9213b..f109c16fd 100644 --- a/program/include/rcmail.php +++ b/program/include/rcmail.php @@ -47,7 +47,7 @@ class rcmail /** * This implements the 'singleton' design pattern * - * @return object qvert The one and only instance + * @return object rcmail The one and only instance */ static function get_instance() { diff --git a/program/steps/mail/func.inc b/program/steps/mail/func.inc index 702625cea..79e148d8a 100644 --- a/program/steps/mail/func.inc +++ b/program/steps/mail/func.inc @@ -21,7 +21,7 @@ require_once('include/rcube_smtp.inc'); -$EMAIL_ADDRESS_PATTERN = '/([a-z0-9][a-z0-9\-\.\+\_]*@[a-z0-9]([a-z0-9\-][.]?)*[a-z0-9]\\.[a-z]{2,5})/i'; +$EMAIL_ADDRESS_PATTERN = '([a-z0-9][a-z0-9\-\.\+\_]*@[a-z0-9]([a-z0-9\-][.]?)*[a-z0-9]\\.[a-z]{2,5})'; // actions that do not require imap connection $NOIMAP_ACTIONS = array('spell', 'addcontact', 'autocomplete', 'upload', 'display-attachment', 'remove-attachment'); @@ -707,7 +707,6 @@ function rcmail_wash_html($html, $p = array(), $cid_replaces) } $washer = new washtml($wash_opts); - $washer->add_callback('a', 'rcmail_washtml_callback'); $washer->add_callback('form', 'rcmail_washtml_callback'); if ($p['safe']) { // allow CSS styles, will be sanitized by rcmail_washtml_callback() @@ -820,11 +819,6 @@ function rcmail_washtml_callback($tagname, $attrib, $content) $out = html::div('form', $content); break; - case 'a': - if ($attrib) $attrib .= ' target="_blank"'; - $out = '' . $content . ''; - break; - case 'style': // decode all escaped entities and reduce to ascii strings $stripped = preg_replace('/[^a-zA-Z\(:]/', '', rcmail_xss_entitiy_decode($content)); @@ -1040,7 +1034,9 @@ function rcmail_html4inline($body, $container_id) } // modify HTML links to open a new window if clicked - $body = preg_replace('/<(a|link)\s+([^>]+)>/Uie', "rcmail_alter_html_link('\\1','\\2', '$container_id');", $body); + $GLOBALS['rcmail_html_container_id'] = $container_id; + $body = preg_replace_callback('/<(a|link)\s+([^>]+)>/Ui', 'rcmail_alter_html_link', $body); + unset($GLOBALS['rcmail_html_container_id']); // add comments arround html and other tags $out = preg_replace(array( @@ -1068,20 +1064,24 @@ function rcmail_html4inline($body, $container_id) /** * parse link attributes and set correct target */ -function rcmail_alter_html_link($tag, $attrs, $container_id) +function rcmail_alter_html_link($matches) { - $attrib = parse_attrib_string($attrs); + global $EMAIL_ADDRESS_PATTERN; + + $tag = $matches[1]; + $attrib = parse_attrib_string($matches[2]); $end = '>'; if ($tag == 'link' && preg_match('/^https?:\/\//i', $attrib['href'])) { - $attrib['href'] = "./bin/modcss.php?u=" . urlencode($attrib['href']) . "&c=" . urlencode($container_id); + $attrib['href'] = "./bin/modcss.php?u=" . urlencode($attrib['href']) . "&c=" . urlencode($GLOBALS['rcmail_html_container_id']); $end = ' />'; } - else if (stristr((string)$attrib['href'], 'mailto:')) { + else if (preg_match("/^mailto:$EMAIL_ADDRESS_PATTERN/i", $attrib['href'], $mailto)) { + $attrib['href'] = $mailto[0]; $attrib['onclick'] = sprintf( "return %s.command('compose','%s',this)", JS_OBJECT_NAME, - JQ(substr($attrib['href'], 7))); + JQ($mailto[1])); } else if (!empty($attrib['href']) && $attrib['href'][0] != '#') { $attrib['target'] = '_blank'; @@ -1112,7 +1112,7 @@ function rcmail_address_string($input, $max=null, $linked=false, $addicon=null) if ($PRINT_MODE) { $out .= sprintf('%s <%s>', Q($part['name']), $part['mailto']); } - else if (preg_match($EMAIL_ADDRESS_PATTERN, $part['mailto'])) { + else if (preg_match("/$EMAIL_ADDRESS_PATTERN/i", $part['mailto'])) { if ($linked) { $out .= html::a(array( 'href' => 'mailto:'.$part['mailto'], diff --git a/tests/mailfunc.php b/tests/mailfunc.php index bf9163b7d..8fd2cd3d5 100644 --- a/tests/mailfunc.php +++ b/tests/mailfunc.php @@ -19,6 +19,8 @@ class rcube_test_mailfunc extends UnitTestCase $IMAP = $RCMAIL->imap; require_once 'steps/mail/func.inc'; + + $GLOBALS['EMAIL_ADDRESS_PATTERN'] = $EMAIL_ADDRESS_PATTERN; } /** @@ -43,7 +45,7 @@ class rcube_test_mailfunc extends UnitTestCase $part->replaces = array('ex1.jpg' => 'part_1.2.jpg', 'ex2.jpg' => 'part_1.2.jpg'); // render HTML in normal mode - $html = rcmail_print_body($part, array('safe' => false)); + $html = rcmail_html4inline(rcmail_print_body($part, array('safe' => false)), 'foo'); $this->assertPattern('/src="'.$part->replaces['ex1.jpg'].'"/', $html, "Replace reference to inline image"); $this->assertPattern('#background="./program/blocked.gif"#', $html, "Replace external background image"); @@ -71,10 +73,13 @@ class rcube_test_mailfunc extends UnitTestCase { $part = $this->get_html_part('src/htmlxss.txt'); $washed = rcmail_print_body($part, array('safe' => true)); - + $this->assertNoPattern('/src="skins/', $washed, "Remove local references"); - $this->assertNoPattern('/\son[a-z]+/', $wahsed, "Remove on* attributes"); - $this->assertNoPattern('/alert/', $wahsed, "Remove alerts"); + $this->assertNoPattern('/\son[a-z]+/', $washed, "Remove on* attributes"); + + $html = rcmail_html4inline($washed, 'foo'); + $this->assertNoPattern('/onclick="return rcmail.command(\'compose\',\'xss@somehost.net\',this)"/', $html, "Clean mailto links"); + $this->assertNoPattern('/alert/', $html, "Remove alerts"); } /** diff --git a/tests/src/htmlxss.txt b/tests/src/htmlxss.txt index 60ceb944e..f6c43e353 100644 --- a/tests/src/htmlxss.txt +++ b/tests/src/htmlxss.txt @@ -3,7 +3,7 @@

-

mail me! +

mail me! roundcube.net roundcube.net (2) -- cgit v1.2.3