thomascube
2009-03-02 11526305f506245af55e8ae7ea31faec49dfd98d
Revert r2322; this is done in rcmail_html4inline() and now secured + fix tests

4 files modified
45 ■■■■■ changed files
program/include/rcmail.php 2 ●●● patch | view | raw | blame | history
program/steps/mail/func.inc 28 ●●●● patch | view | raw | blame | history
tests/mailfunc.php 13 ●●●●● patch | view | raw | blame | history
tests/src/htmlxss.txt 2 ●●● patch | view | raw | blame | history
program/include/rcmail.php
@@ -47,7 +47,7 @@
  /**
   * 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()
  {
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 @@
  }
    
  $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()
@@ -818,11 +817,6 @@
  switch ($tagname) {
    case 'form':
      $out = html::div('form', $content);
      break;
    case 'a':
      if ($attrib) $attrib .= ' target="_blank"';
      $out = '<a'.$attrib.'>' . $content . '</a>';
      break;
      
    case 'style':
@@ -1040,7 +1034,9 @@
    }
  // 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 @@
/**
 * 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']) . "&amp;c=" . urlencode($container_id);
    $attrib['href'] = "./bin/modcss.php?u=" . urlencode($attrib['href']) . "&amp;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 @@
    if ($PRINT_MODE) {
      $out .= sprintf('%s &lt;%s&gt;', 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'],
tests/mailfunc.php
@@ -19,6 +19,8 @@
    $IMAP = $RCMAIL->imap;
    
    require_once 'steps/mail/func.inc';
    $GLOBALS['EMAIL_ADDRESS_PATTERN'] = $EMAIL_ADDRESS_PATTERN;
  }
  /**
@@ -43,7 +45,7 @@
    $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 @@
  {
    $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");
  }
  /**
tests/src/htmlxss.txt
@@ -3,7 +3,7 @@
<p><img onLoad.="alert(document.cookie)" src="skins/default/images/roundcube_logo.png" /></p>
<p><a href="javascript:alert(document.cookie)">mail me!</a>
<p><a href="mailto:xss@somehost.net') && alert(document.cookie) || ignore('">mail me!</a>
<a href="http://roundcube.net" target="_self">roundcube.net</a>
<a href="http://roundcube.net" \onmouseover="alert('XSS')">roundcube.net (2)</a>