Get rid of vulnerable preg_replace eval and create_function (#1485686) + correctly handle base and link tags in html messages

author: thomascube <thomas@roundcube.net> 2009-01-22 14:47:23 +0000
committer: thomascube <thomas@roundcube.net> 2009-01-22 14:47:23 +0000
commit: aa055c931a68547763f7bb89425a08e8ceecb749 (patch)
tree: ece66b96ce0bc8c1f892f6c620a32c93f5df621f
parent: 4f27148d400661c81005b496ac7c05b6c6ed9483 (diff)
5 files changed, 191 insertions, 45 deletions
diff --git a/CHANGELOG b/CHANGELOG
index e8ce8272a..123b24377 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,11 @@
 CHANGELOG RoundCube Webmail
 ---------------------------
 
+2009/01/22 (thomasb)
+----------
+- Get rid of preg_replace() with eval modifier and create_function usage (#1485686)
+- Bring back <base> and <link> tags in HTML messages
+
 2009/01/20 (thomasb)
 ----------
 - Fix XSS vulnerability through background attributes as reported by Julien Cayssol
diff --git a/program/include/main.inc b/program/include/main.inc
index bdf711633..5ee652269 100644
--- a/program/include/main.inc
+++ b/program/include/main.inc
@@ -587,25 +587,24 @@ function rcmail_get_edit_field($col, $value, $attrib, $type='text')
  * @param string Container ID to use as prefix
  * @return string Modified CSS source
  */
-function rcmail_mod_css_styles($source, $container_id, $base_url = '')
+function rcmail_mod_css_styles($source, $container_id)
   {
-  $a_css_values = array();
   $last_pos = 0;
+  $replacements = new rcube_string_replacer;
   
   // ignore the whole block if evil styles are detected
   $stripped = preg_replace('/[^a-z\(:]/', '', rcmail_xss_entitiy_decode($source));
   if (preg_match('/expression|behavior|url\(|import/', $stripped))
-    return '';
+    return '/* evil! */';
 
   // cut out all contents between { and }
   while (($pos = strpos($source, '{', $last_pos)) && ($pos2 = strpos($source, '}', $pos)))
   {
-    $key = sizeof($a_css_values);
-    $a_css_values[$key] = substr($source, $pos+1, $pos2-($pos+1));
-    $source = substr($source, 0, $pos+1) . "<<str_replacement[$key]>>" . substr($source, $pos2, strlen($source)-$pos2);
+    $key = $replacements->add(substr($source, $pos+1, $pos2-($pos+1)));
+    $source = substr($source, 0, $pos+1) . $replacements->get_replacement($key) . substr($source, $pos2, strlen($source)-$pos2);
     $last_pos = $pos+2;
   }
-
+  
   // remove html comments and add #container to each tag selector.
   // also replace body definition because we also stripped off the <body> tag
   $styles = preg_replace(
@@ -621,17 +620,8 @@ function rcmail_mod_css_styles($source, $container_id, $base_url = '')
     ),
     $source);
   
-  // replace all @import statements to modify the imported CSS sources too
-  $styles = preg_replace_callback(
-    '/@import\s+(url\()?[\'"]?([^\)\'"]+)[\'"]?(\))?/im',
-    create_function('$matches', "return sprintf(\"@import url('./bin/modcss.php?u=%s&c=%s')\", urlencode(make_absolute_url(\$matches[2],'$base_url')), urlencode('$container_id'));"),
-    $styles);
-  
   // put block contents back in
-  $styles = preg_replace_callback(
-    '/<<str_replacement\[([0-9]+)\]>>/',
-    create_function('$matches', "\$values = ".var_export($a_css_values, true)."; return \$values[\$matches[1]];"),
-    $styles);
+  $styles = $replacements->resolve($styles);
 
   return $styles;
   }
@@ -647,13 +637,24 @@ function rcmail_mod_css_styles($source, $container_id, $base_url = '')
 function rcmail_xss_entitiy_decode($content)
 {
   $out = html_entity_decode(html_entity_decode($content));
-  $out = preg_replace_callback('/\\\([0-9a-f]{4})/i', create_function('$matches', 'return chr(hexdec($matches[1]));'), $out);
+  $out = preg_replace_callback('/\\\([0-9a-f]{4})/i', 'rcmail_xss_entitiy_decode_callback', $out);
   $out = preg_replace('#/\*.*\*/#Um', '', $out);
   return $out;
 }
 
 
 /**
+ * preg_replace_callback callback for rcmail_xss_entitiy_decode_callback
+ *
+ * @param array matches result from preg_replace_callback
+ * @return string decoded entity
+ */ 
+function rcmail_xss_entitiy_decode_callback($matches)
+{ 
+  return chr(hexdec($matches[1]));
+}
+
+/**
  * Compose a valid attribute string for HTML tags
  *
  * @param array Named tag attributes
@@ -1209,4 +1210,26 @@ function rcube_html_editor($mode='')
   $OUTPUT->add_script('rcmail_editor_init("$__skin_path", "'.JQ($tinylang).'", '.intval($CONFIG['enable_spellcheck']).', "'.$mode.'");');
 }
 
+
+
+/**
+ * Helper class to turn relative urls into absolute ones
+ * using a predefined base
+ */
+class rcube_base_replacer
+{
+  private $base_url;
+  
+  public function __construct($base)
+  {
+    $this->base_url = $base;
+  }
+  
+  public function callback($matches)
+  {
+    return $matches[1] . '="' . make_absolute_url($matches[3], $this->base_url) . '"';
+  }
+}
+
+
 ?>
diff --git a/program/include/rcube_shared.inc b/program/include/rcube_shared.inc
index 244225ca9..c1c056109 100644
--- a/program/include/rcube_shared.inc
+++ b/program/include/rcube_shared.inc
@@ -309,7 +309,7 @@ function make_absolute_url($path, $base_url)
     return $path;
 
   // cut base_url to the last directory
-  if (strpos($base_url, '/')>7)
+  if (strrpos($base_url, '/')>7)
   {
     $host_url = substr($base_url, 0, strpos($base_url, '/'));
     $base_url = substr($base_url, 0, strrpos($base_url, '/'));
diff --git a/program/include/rcube_string_replacer.php b/program/include/rcube_string_replacer.php
new file mode 100644
index 000000000..fe082a583
--- /dev/null
+++ b/program/include/rcube_string_replacer.php
@@ -0,0 +1,116 @@
+<?php
+
+/*
+ +-----------------------------------------------------------------------+
+ | program/include/rcube_string_replacer.php                             |
+ |                                                                       |
+ | This file is part of the RoundCube Webmail client                     |
+ | Copyright (C) 2009, RoundCube Dev. - Switzerland                      |
+ | Licensed under the GNU GPL                                            |
+ |                                                                       |
+ | PURPOSE:                                                              |
+ |   Handle string replacements based on preg_replace_callback           |
+ |                                                                       |
+ +-----------------------------------------------------------------------+
+ | Author: Thomas Bruederli <roundcube@gmail.com>                        |
+ +-----------------------------------------------------------------------+
+
+ $Id:  $
+
+*/
+
+
+/**
+ * Helper class for string replacements based on preg_replace_callback
+ *
+ * @package Core
+ */
+class rcube_string_replacer
+{
+  public static $pattern = '/##str_replacement\[([0-9]+)\]##/';
+
+  private $values = array();
+
+
+  /**
+   * Add a string to the internal list
+   *
+   * @param string String value 
+   * @return int Index of value for retrieval
+   */
+  public function add($str)
+  {
+    $i = count($this->values);
+    $this->values[$i] = $str;
+    return $i;
+  }
+
+  /**
+   * Build replacement string
+   */
+  public function get_replacement($i)
+  {
+    return '##str_replacement['.$i.']##';
+  }
+
+  /**
+   * Callback function used to build HTML links around URL strings
+   *
+   * @param array Matches result from preg_replace_callback
+   * @return int Index of saved string value
+   */
+  public function link_callback($matches)
+  {
+    $i = -1;
+    $scheme = strtolower($matches[1]);
+
+    if ($scheme == 'http' || $scheme == 'https' || $scheme == 'ftp') {
+      $url = $matches[1] . '://' . $matches[2];
+      $i = $this->add(html::a(array('href' => $url, 'target' => "_blank"), Q($url)));
+    }
+    else if ($matches[2] == 'www.') {
+      $url = $matches[2] . $matches[3];
+      $i = $this->add($matches[1] . html::a(array('href' => 'http://' . $url, 'target' => "_blank"), Q($url)));
+    }
+
+    return $i >= 0 ? $this->get_replacement($i) : '';
+  }
+
+  /**
+   * Callback function used to build mailto: links around e-mail strings
+   *
+   * @param array Matches result from preg_replace_callback
+   * @return int Index of saved string value
+   */
+  public function mailto_callback($matches)
+  {
+    $i = $this->add(html::a(array(
+        'href' => 'mailto:' . $matches[1],
+        'onclick' => "return ".JS_OBJECT_NAME.".command('compose','".JQ($matches[1])."',this)",
+      ),
+      Q($matches[1])));
+
+    return $i >= 0 ? $this->get_replacement($i) : '';
+  }
+
+  /**
+   * Look up the index from the preg_replace matches array
+   * and return the substitution value.
+   *
+   * @param array Matches result from preg_replace_callback
+   * @return string Value at index $matches[1]
+   */
+  public function replace_callback($matches)
+  {
+    return $this->values[$matches[1]];
+  }
+
+  /**
+   * Replace substituted strings with original values
+   */
+  public function resolve($str)
+  {
+    return preg_replace_callback(self::$pattern, array($this, 'replace_callback'), $str);
+  }
+
+}
+\ No newline at end of file
diff --git a/program/steps/mail/func.inc b/program/steps/mail/func.inc
index 3a5e13f5a..aad60f677 100644
--- a/program/steps/mail/func.inc
+++ b/program/steps/mail/func.inc
@@ -671,6 +671,9 @@ function rcmail_print_body($part, $p = array())
         $html = '<head></head>'. $html;
       $html = substr_replace($html, '<meta http-equiv="content-type" content="text/html; charset='.RCMAIL_CHARSET.'" />', intval(stripos($html, '<head>')+6), 0);
     }
+    
+    // turn relative into absolute urls
+    $html = rcmail_resolve_base($html);
 
     // clean HTML with washhtml by Frederic Motte
     $wash_opts = array(
@@ -685,6 +688,9 @@ function rcmail_print_body($part, $p = array())
     if (!$p['inline_html']) {
       $wash_opts['html_elements'] = array('html','head','title','body');
     }
+    if ($p['safe']) {
+      $wash_opts['html_elements'][] = 'link';
+    }
     
     $washer = new washtml($wash_opts);
     $washer->add_callback('form', 'rcmail_washtml_callback');
@@ -710,22 +716,15 @@ function rcmail_print_body($part, $p = array())
   /**** assert plaintext ****/
 
   // make links and email-addresses clickable
-  $convert_patterns = $convert_replaces = $replace_strings = array();
+  $replacements = new rcube_string_replacer;
   
   $url_chars = 'a-z0-9_\-\+\*\$\/&%=@#:;';
   $url_chars_within = '\?\.~,!';
-
-  $convert_patterns[] = "/([\w]+):\/\/([a-z0-9\-\.]+[a-z]{2,4}([$url_chars$url_chars_within]*[$url_chars])?)/ie";
-  $convert_replaces[] = "rcmail_str_replacement('<a href=\"\\1://\\2\" target=\"_blank\">\\1://\\2</a>', \$replace_strings)";
-
-  $convert_patterns[] = "/([^\/:]|\s)(www\.)([a-z0-9\-]{2,}[a-z]{2,4}([$url_chars$url_chars_within]*[$url_chars])?)/ie";
-  $convert_replaces[] = "rcmail_str_replacement('\\1<a href=\"http://\\2\\3\" target=\"_blank\">\\2\\3</a>', \$replace_strings)";
-  
-  $convert_patterns[] = '/([a-z0-9][a-z0-9\-\.\+\_]*@[a-z0-9]([a-z0-9\-][.]?)*[a-z0-9]\\.[a-z]{2,5})/ie';
-  $convert_replaces[] = "rcmail_str_replacement('<a href=\"mailto:\\1\" onclick=\"return ".JS_OBJECT_NAME.".command(\'compose\',\'\\1\',this)\">\\1</a>', \$replace_strings)";
   
   // search for patterns like links and e-mail addresses
-  $body = preg_replace($convert_patterns, $convert_replaces, $body);
+  $body = preg_replace_callback("/([\w]+):\/\/([a-z0-9\-\.]+[a-z]{2,4}([$url_chars$url_chars_within]*[$url_chars])?)/i", array($replacements, 'link_callback'), $body);
+  $body = preg_replace_callback("/([^\/:]|\s)(www\.)([a-z0-9\-]{2,}[a-z]{2,4}([$url_chars$url_chars_within]*[$url_chars])?)/i", array($replacements, 'link_callback'), $body);
+  $body = preg_replace_callback('/([a-z0-9][a-z0-9\-\.\+\_]*@[a-z0-9]([a-z0-9\-][.]?)*[a-z0-9]\\.[a-z]{2,5})/i', array($replacements, 'mailto_callback'), $body);
 
   // split body into single lines
   $a_lines = preg_split('/\r?\n/', $body);
@@ -754,7 +753,7 @@ function rcmail_print_body($part, $p = array())
   }
 
   // insert the links for urls and mailtos
-  $body = preg_replace("/##string_replacement\{([0-9]+)\}##/e", "\$replace_strings[\\1]", join("\n", $a_lines));
+  $body = $replacements->resolve(join("\n", $a_lines));
 
   return html::tag('pre', array(), $body);
 }
@@ -956,41 +955,44 @@ function rcmail_message_body($attrib)
   }
 
 
+/**
+ * Convert all relative URLs according to a <base> in HTML
+ */
+function rcmail_resolve_base($body)
+{
+  // check for <base href=...>
+  if (preg_match('!(<base.*href=["\']?)([hftps]{3,5}://[a-z0-9/.%-]+)!i', $body, $regs)) {
+    $replacer = new rcube_base_replacer($regs[2]);
+
+    // replace all relative paths
+    $body = preg_replace_callback('/(src|background|href)=(["\']?)([\.\/]+[^"\'\s]+)(\2|\s|>)/Ui', array($replacer, 'callback'), $body);
+    $body = preg_replace_callback('/(url\s*\()(["\']?)([\.\/]+[^"\'\)\s]+)(\2)\)/Ui', array($replacer, 'callback'), $body);
+  }
+
+  return $body;
+}
 
 /**
  * modify a HTML message that it can be displayed inside a HTML page
  */
 function rcmail_html4inline($body, $container_id)
   {
-  $base_url = "";
   $last_style_pos = 0;
   $body_lc = strtolower($body);
   
-  // check for <base href>
-  if (preg_match(($base_reg = '/(<base.*href=["\']?)([hftps]{3,5}:\/{2}[^"\'\s]+)([^<]*>)/i'), $body, $base_regs))
-    $base_url = $base_regs[2];
-  
   // find STYLE tags
   while (($pos = strpos($body_lc, '<style', $last_style_pos)) && ($pos2 = strpos($body_lc, '</style>', $pos)))
     {
     $pos = strpos($body_lc, '>', $pos)+1;
 
     // replace all css definitions with #container [def]
-    $styles = rcmail_mod_css_styles(substr($body, $pos, $pos2-$pos), $container_id, $base_url);
+    $styles = rcmail_mod_css_styles(substr($body, $pos, $pos2-$pos), $container_id);
 
     $body = substr($body, 0, $pos) . $styles . substr($body, $pos2);
     $body_lc = strtolower($body);
     $last_style_pos = $pos2;
     }
 
-  // resolve <base href>
-  if ($base_url)
-    {
-    $body = preg_replace('/(src|background|href)=(["\']?)([\.\/]+[^"\'\s]+)(\2|\s|>)/Uie', "'\\1=\"'.make_absolute_url('\\3', '$base_url').'\"'", $body);
-    $body = preg_replace('/(url\s*\()(["\']?)([\.\/]+[^"\'\)\s]+)(\2)\)/Uie', "'\\1\''.make_absolute_url('\\3', '$base_url').'\')'", $body);
-    $body = preg_replace($base_reg, '', $body);
-    }
-    
   // 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);
author	thomascube <thomas@roundcube.net>	2009-01-22 14:47:23 +0000
committer	thomascube <thomas@roundcube.net>	2009-01-22 14:47:23 +0000
commit	aa055c931a68547763f7bb89425a08e8ceecb749 (patch)
tree	ece66b96ce0bc8c1f892f6c620a32c93f5df621f
parent	4f27148d400661c81005b496ac7c05b6c6ed9483 (diff)