Changeset 5591 in subversion

Timestamp:

Dec 10, 2011 9:16:31 AM (18 months ago)

Author:

thomasb

Message:

Backported CSS sanitization (r5586:r5590)

Location:

branches/release-0.7

Files:

• CHANGELOG (modified) (1 diff)
• program/include/main.inc (modified) (3 diffs)
• program/lib/washtml.php (modified) (3 diffs)
• program/steps/mail/func.inc (modified) (6 diffs)

branches/release-0.7/CHANGELOG

@@ -4 +4 @@
 RELEASE 0.7
 -----------
+- Make Roundcube render the Email Standards Project Acid Test correctly
 - Replace prompt() with jQuery UI dialog (#1485135)
 - Fix navigation in messages search results

branches/release-0.7/program/include/main.inc

@@ -884 +884 @@
  * @return string Modified CSS source
  */
-function rcmail_mod_css_styles($source, $container_id)
+function rcmail_mod_css_styles($source, $container_id, $allow_remote=false)
   {
   $last_pos = 0;
@@ -890 +890 @@
 
   // ignore the whole block if evil styles are detected
-  $stripped = preg_replace('/[^a-z\(:;]/', '', rcmail_xss_entity_decode($source));
-  if (preg_match('/expression|behavior|url\(|import[^a]/', $stripped))
+  $source = rcmail_xss_entity_decode($source);
+  $stripped = preg_replace('/[^a-z\(:;]/i', '', $source);
+  $evilexpr = 'expression|behavior|javascript:|import[^a]' . (!$allow_remote ? '|url\(' : '');
+  if (preg_match("/$evilexpr/i", $stripped))
     return '/* evil! */';
 
-  // remove css comments (sometimes used for some ugly hacks)
-  $source = preg_replace('!/\*(.+)\*/!Ums', '', $source);
-
   // cut out all contents between { and }
-  while (($pos = strpos($source, '{', $last_pos)) && ($pos2 = strpos($source, '}', $pos)))
-  {
-    $key = $replacements->add(substr($source, $pos+1, $pos2-($pos+1)));
+  while (($pos = strpos($source, '{', $last_pos)) && ($pos2 = strpos($source, '}', $pos))) {
+    $styles = substr($source, $pos+1, $pos2-($pos+1));
+
+    // check every line of a style block...
+    if ($allow_remote) {
+      $a_styles = preg_split('/;[\r\n]*/', $styles, -1, PREG_SPLIT_NO_EMPTY);
+      foreach ($a_styles as $line) {
+        $stripped = preg_replace('/[^a-z\(:;]/i', '', $line);
+        // ... and only allow strict url() values
+        if (stripos($stripped, 'url(') && !preg_match('!url\s*\([ "\'](https?:)//[a-z0-9/._+-]+["\' ]\)!Uims', $line)) {
+          $a_styles = array('/* evil! */');
+          break;
+        }
+      }
+      $styles = join(";\n", $a_styles);
+    }
+
+    $key = $replacements->add($styles);
     $source = substr($source, 0, $pos+1) . $replacements->get_replacement($key) . substr($source, $pos2, strlen($source)-$pos2);
     $last_pos = $pos+2;
@@ -938 +952 @@
   $out = html_entity_decode(html_entity_decode($content));
   $out = preg_replace_callback('/\\\([0-9a-f]{4})/i', 'rcmail_xss_entity_decode_callback', $out);
-  $out = preg_replace('#/\*.*\*/#Um', '', $out);
+  $out = preg_replace('#/\*.*\*/#Ums', '', $out);
   return $out;
 }

branches/release-0.7/program/lib/washtml.php

@@ -169 +169 @@
               $value .= ' url('.htmlspecialchars($src, ENT_QUOTES) . ')';
             }
-            else if (preg_match('/^(http|https|ftp):.*$/i', $match[2], $url)) {
+            else if (preg_match('!^(https?:)?//[a-z0-9/._+-]+$!i', $match[2], $url)) {
               if ($this->config['allow_remote'])
                 $value .= ' url('.htmlspecialchars($url[0], ENT_QUOTES).')';
@@ -244 +244 @@
         $tagName = strtolower($node->tagName);
         if ($callback = $this->handlers[$tagName]) {
-          $dump .= call_user_func($callback, $tagName, $this->wash_attribs($node), $this->dumpHtml($node));
+          $dump .= call_user_func($callback, $tagName, $this->wash_attribs($node), $this->dumpHtml($node), $this);
         }
         else if (isset($this->_html_elements[$tagName])) {
@@ -302 +302 @@
   }
 
+  /**
+   * Getter for config parameters
+   */
+  public function get_config($prop)
+  {
+      return $this->config[$prop];
+  }
+
 }
 

branches/release-0.7/program/steps/mail/func.inc

@@ -822 +822 @@
  * Callback function for washtml cleaning class
  */
-function rcmail_washtml_callback($tagname, $attrib, $content)
+function rcmail_washtml_callback($tagname, $attrib, $content, $washtml)
 {
   switch ($tagname) {
@@ -834 +834 @@
 
       // now check for evil strings like expression, behavior or url()
-      if (!preg_match('/expression|behavior|url\(|import[^a]/', $stripped)) {
-        $out = html::tag('style', array('type' => 'text/css'), $content);
+      if (!preg_match('/expression|behavior|javascript:|import[^a]/i', $stripped)) {
+        if (!$washtml->get_config('allow_remote') && stripos($stripped, 'url('))
+          $washtml->extlinks = true;
+        else
+          $out = html::tag('style', array('type' => 'text/css'), $content);
         break;
       }
@@ -1015 +1018 @@
 
         if ($part->ctype_secondary == 'html') {
-          $body = rcmail_html4inline($body, $attrib['id'], 'rcmBody', $attrs);
+          $body = rcmail_html4inline($body, $attrib['id'], 'rcmBody', $attrs, $safe_mode);
           $div_attr = array('class' => 'message-htmlpart');
           $style = array();
@@ -1089 +1092 @@
  * modify a HTML message that it can be displayed inside a HTML page
  */
-function rcmail_html4inline($body, $container_id, $body_id='', &$attributes=null)
+function rcmail_html4inline($body, $container_id, $body_id='', &$attributes=null, $allow_remote=false)
 {
   $last_style_pos = 0;
@@ -1102 +1105 @@
     // replace all css definitions with #container [def]
     $styles = rcmail_mod_css_styles(
-      substr($body, $pos, $pos2-$pos), $cont_id);
+      substr($body, $pos, $pos2-$pos), $cont_id, $allow_remote);
 
     $body = substr($body, 0, $pos) . $styles . substr($body, $pos2);
@@ -1280 +1283 @@
             'onclick' => sprintf("return %s.command('add-contact','%s',this)", JS_OBJECT_NAME, urlencode($string)),
             'title' => rcube_label('addtoaddressbook'),
+            'class' => 'rcmaddcontact',
           ),
           html::img(array(