Changeset 2324 in subversion

Timestamp:

Mar 2, 2009 12:34:18 PM (4 years ago)

Author:

thomasb

Message:

Revert r2322; this is done in rcmail_html4inline() and now secured + fix tests

Location:

trunk/roundcubemail

Files:

• program/include/rcmail.php (modified) (1 diff)
• program/steps/mail/func.inc (modified) (6 diffs)
• tests/mailfunc.php (modified) (3 diffs)
• tests/src/htmlxss.txt (modified) (1 diff)

trunk/roundcubemail/program/include/rcmail.php

@@ -48 +48 @@
    * 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()

trunk/roundcubemail/program/steps/mail/func.inc

@@ -22 +22 @@
 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
@@ -708 +708 @@
     
   $washer = new washtml($wash_opts);
-  $washer->add_callback('a', 'rcmail_washtml_callback');
   $washer->add_callback('form', 'rcmail_washtml_callback');
 
@@ -819 +818 @@
     case 'form':
       $out = html::div('form', $content);
-      break;
-      
-    case 'a':
-      if ($attrib) $attrib .= ' target="_blank"';
-      $out = '<a'.$attrib.'>' . $content . '</a>';
       break;
       
@@ -1041 +1035 @@
 
   // 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
@@ -1069 +1065 @@
  * 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] != '#') {
@@ -1113 +1113 @@
       $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(

trunk/roundcubemail/tests/mailfunc.php

@@ -20 +20 @@
     
     require_once 'steps/mail/func.inc';
+    
+    $GLOBALS['EMAIL_ADDRESS_PATTERN'] = $EMAIL_ADDRESS_PATTERN;
   }
 
@@ -44 +46 @@
     
     // 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");
@@ -72 +74 @@
     $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");
   }
 

trunk/roundcubemail/tests/src/htmlxss.txt

@@ -4 +4 @@
 <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>