Changeset 2245 in subversion

Timestamp:

Jan 20, 2009 11:28:33 AM (4 years ago)

Author:

thomasb

Message:

Treat 'background' attributes the same way as 'src' (another XSS vulnerability)

Location:

trunk/roundcubemail

Files:

• CHANGELOG (modified) (1 diff)
• program/lib/washtml.php (modified) (3 diffs)

trunk/roundcubemail/CHANGELOG

@@ -1 +1 @@
 CHANGELOG RoundCube Webmail
 ---------------------------
+
+2009/01/20 (thomasb)
+----------
+- Fix XSS vulnerability through background attributes as reported by Julien Cayssol
 
 2009/01/18 (alec)

trunk/roundcubemail/program/lib/washtml.php

@@ -81 +81 @@
   
   /* Allowed HTML attributes */
-  static $html_attribs = array('name', 'class', 'title', 'alt', 'width', 'height', 'align', 'nowrap', 'col', 'row', 'id', 'rowspan', 'colspan', 'cellspacing', 'cellpadding', 'valign', 'bgcolor', 'color', 'border', 'bordercolorlight', 'bordercolordark', 'face', 'marginwidth', 'marginheight', 'axis', 'border', 'abbr', 'char', 'charoff', 'clear', 'compact', 'coords', 'vspace', 'hspace', 'cellborder', 'size', 'lang', 'dir', 'background');  
+  static $html_attribs = array('name', 'class', 'title', 'alt', 'width', 'height', 'align', 'nowrap', 'col', 'row', 'id', 'rowspan', 'colspan', 'cellspacing', 'cellpadding', 'valign', 'bgcolor', 'color', 'border', 'bordercolorlight', 'bordercolordark', 'face', 'marginwidth', 'marginheight', 'axis', 'border', 'abbr', 'char', 'charoff', 'clear', 'compact', 'coords', 'vspace', 'hspace', 'cellborder', 'size', 'lang', 'dir');  
   
   /* State for linked objects in HTML */
@@ -161 +161 @@
       $value = $node->getAttribute($key);
       if(isset($this->_html_attribs[$key]) ||
-         ($key == 'href' && preg_match('/^(http|https|ftp|mailto):.*/i', $value)))
+         ($key == 'href' && preg_match('/^(http|https|ftp|mailto):.+/i', $value)))
         $t .= ' ' . $key . '="' . htmlspecialchars($value, ENT_QUOTES) . '"';
       else if($key == 'style' && ($style = $this->wash_style($value)))
         $t .= ' style="' . $style . '"';
-      else if($key == 'src' && strtolower($node->tagName) == 'img') { //check tagName anyway
+      else if($key == 'background' || ($key == 'src' && strtolower($node->tagName) == 'img')) { //check tagName anyway
         if($src = $this->config['cid_map'][$value]) {
           $t .= ' ' . $key . '="' . htmlspecialchars($src, ENT_QUOTES) . '"';
         }
-        else if(preg_match('/^(http|https|ftp):.*/i', $value)) {
+        else if(preg_match('/^(http|https|ftp):.+/i', $value)) {
           if($this->config['allow_remote'])
             $t .= ' ' . $key . '="' . htmlspecialchars($value, ENT_QUOTES) . '"';
@@ -175 +175 @@
             $this->extlinks = true;
             if ($this->config['blocked_src'])
-              $t .= ' src="' . htmlspecialchars($this->config['blocked_src'], ENT_QUOTES) . '"';
+              $t .= ' ' . $key . '="' . htmlspecialchars($this->config['blocked_src'], ENT_QUOTES) . '"';
           }
         }