diff --git a/program/steps/mail/func.inc b/program/steps/mail/func.inc
index 0e13c8453..79f911ead 100644
--- a/program/steps/mail/func.inc
+++ b/program/steps/mail/func.inc
@@ -860,6 +860,15 @@ function rcmail_wash_html($html, $p, $cid_replaces)
$washer->add_callback('style', 'rcmail_washtml_callback');
}
+ // modify HTML links to open a new window if clicked
+ if (!$p['skip_washer_link_callback']) {
+ $washer->add_callback('a', 'rcmail_washtml_link_callback');
+ $washer->add_callback('area', 'rcmail_washtml_link_callback');
+
+ if ($p['safe'])
+ $washer->add_callback('link', 'rcmail_washtml_link_callback');
+ }
+
// Remove non-UTF8 characters (#1487813)
$html = rcube_charset::clean($html);
@@ -1427,11 +1436,6 @@ function rcmail_html4inline($body, $container_id, $body_class='', &$attributes=n
$last_style_pos = $pos2 + strlen($styles) - $len;
}
- // modify HTML links to open a new window if clicked
- $GLOBALS['rcmail_html_container_id'] = $container_id;
- $body = preg_replace_callback('/<(a|link|area)\s+([^>]+)>/Ui', 'rcmail_alter_html_link', $body);
- unset($GLOBALS['rcmail_html_container_id']);
-
$body = preg_replace(array(
// add comments arround html and other tags
'/(]*>)/i',
@@ -1510,13 +1514,11 @@ function rcmail_html4inline($body, $container_id, $body_class='', &$attributes=n
/**
* parse link (a, link, area) attributes and set correct target
*/
-function rcmail_alter_html_link($matches)
+function rcmail_washtml_link_callback($tag, $attribs, $content, $washtml)
{
global $RCMAIL;
- $tag = strtolower($matches[1]);
- $attrib = html::parse_attrib_string($matches[2]);
- $end = '>';
+ $attrib = html::parse_attrib_string($attribs);
// Remove non-printable characters in URL (#1487805)
if ($attrib['href'])
@@ -1559,7 +1561,7 @@ function rcmail_alter_html_link($matches)
$attrib['onclick'] = '';
}
}
- else if (empty($attrib['href']) && !$attrib['name']) {
+ else if (empty($attrib['href']) && !isset($attrib['name'])) {
$attrib['href'] = './#NOP';
$attrib['onclick'] = 'return false';
}
@@ -1576,7 +1578,7 @@ function rcmail_alter_html_link($matches)
$allow = array('href','name','target','onclick','id','class','style','title',
'rel','type','media','alt','coords','nohref','hreflang','shape');
- return "<$tag" . html::attrib_string($attrib, $allow) . $end;
+ return html::tag($tag, $attrib, $content, $allow);
}
/**
diff --git a/tests/MailFunc.php b/tests/MailFunc.php
index f4fc3ed37..69b8a3517 100644
--- a/tests/MailFunc.php
+++ b/tests/MailFunc.php
@@ -22,12 +22,12 @@ class MailFunc extends PHPUnit_Framework_TestCase
/**
* Helper method to create a HTML message part object
*/
- function get_html_part($body)
+ function get_html_part($body = null)
{
$part = new rcube_message_part;
$part->ctype_primary = 'text';
$part->ctype_secondary = 'html';
- $part->body = file_get_contents(TESTS_DIR . $body);
+ $part->body = $body ? file_get_contents(TESTS_DIR . $body) : null;
$part->replaces = array();
return $part;
}
@@ -184,6 +184,40 @@ class MailFunc extends PHPUnit_Framework_TestCase
$this->assertRegExp('|src="http://other\.domain\.tld/img3\.gif"|', $html, "URI base resolving exception [2]");
}
+ /**
+ * Test link attribute modifications
+ */
+ public function test_html_links()
+ {
+ // disable relative links
+ $html = 'test';
+ $body = rcmail_print_body($html, $this->get_html_part(), array('safe' => false, 'plain' => false));
+
+ $this->assertNotContains('href="/"', $body);
+ $this->assertContains('assertContains('onclick="return false"', $body);
+
+ $html = 'test';
+ $body = rcmail_print_body($html, $this->get_html_part(), array('safe' => false, 'plain' => false));
+
+ // allow external links, add target and noreferrer
+ $this->assertContains('assertContains(' target="_blank"', $body);
+ $this->assertContains(' rel="noreferrer"', $body);
+ }
+
+ /**
+ * Test potential XSS with invalid attributes
+ */
+ public function test_html_link_xss()
+ {
+ $html = 'test';
+ $body = rcmail_print_body($html, $this->get_html_part(), array('safe' => false, 'plain' => false));
+
+ $this->assertNotContains('onerror=alert(1)//">test', $body);
+ $this->assertContains('