Modify links in html messages during Washtml DOM traversal

This is a more safe approach than using regex and mitigates
possible vulnerabilities using malformed html markup.
pull/5899/merge
Thomas Bruederli 7 years ago
parent 919338d4ba
commit 1cfc024036

@ -903,6 +903,15 @@ function rcmail_wash_html($html, $p, $cid_replaces = array())
$washer->add_callback('style', 'rcmail_washtml_callback');
}
// alter link tag attributes
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);
@ -1478,7 +1487,6 @@ function rcmail_html4inline($body, $container_id, $body_class='', &$attributes=a
// 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(
@ -1566,13 +1574,11 @@ function rcmail_html4inline($body, $container_id, $body_class='', &$attributes=a
/**
* 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']) {
@ -1616,7 +1622,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';
}
@ -1633,7 +1639,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);
}
/**

@ -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 = '<a href="/">test</a>';
$body = rcmail_print_body($html, $this->get_html_part(), array('safe' => false, 'plain' => false));
$this->assertNotContains('href="/"', $body);
$this->assertContains('<a href="./#NOP"', $body);
$this->assertContains('onclick="return false"', $body);
$html = '<a href="https://roundcube.net">test</a>';
$body = rcmail_print_body($html, $this->get_html_part(), array('safe' => false, 'plain' => false));
// allow external links, add target and noreferrer
$this->assertContains('<a href="https://roundcube.net"', $body);
$this->assertContains(' target="_blank"', $body);
$this->assertContains(' rel="noreferrer"', $body);
}
/**
* Test potential XSS with invalid attributes
*/
public function test_html_link_xss()
{
$html = '<a style="x:><img src=x onerror=alert(1)//">test</a>';
$body = rcmail_print_body($html, $this->get_html_part(), array('safe' => false, 'plain' => false));
$this->assertNotContains('onerror=alert(1)//">test', $body);
$this->assertContains('<a style="x: &gt;&lt;img src=x onerror=alert(1)//"', $body);
}
/**
* Test identities selection using Return-Path header
*/

Loading…
Cancel
Save