- added Smarty_Security::isTrustedUri() and Smarty_Security::$trusted_uri to validate remote resource calls through {fetch} and {html_image} (Forum Topic 20627)

change_log.txt

@@ -6,6 +6,8 @@
 - bugfix unregisterObject() raised notice when object to unregister did not exist
 - changed internals to use Smarty::$_MBSTRING ($_CHARSET, $_DATE_FORMAT) for better unit testing
 - added Smarty::$_UTF8_MODIFIER for proper PCRE charset handling (Forum Topic 20452)
+- added Smarty_Security::isTrustedUri() and Smarty_Security::$trusted_uri to validate 
+  remote resource calls through {fetch} and {html_image} (Forum Topic 20627)
 
 17.12.2011
 - improvement of compiling speed by new handling of plain text blocks in the lexer/parser (issue #68)

libs/plugins/function.fetch.php

@@ -27,25 +27,32 @@ function smarty_function_fetch($params, $template)
         return;
     }
     
-    $content = '';
+    // strip file protocol
-    if (isset($template->smarty->security_policy) && !preg_match('!^(http|ftp)://!i', $params['file'])) {
+    if (stripos($params['file'], 'file://') === 0) {
+        $params['file'] = substr($params['file'], 7);
+    }
+    
+    $protocol = strpos($params['file'], '://');
+    if ($protocol !== false) {
+        $protocol = strtolower(substr($params['file'], 0, $protocol));
+    }
+    
+    if (isset($template->smarty->security_policy)) {
+        if ($protocol) {
+            // remote resource (or php stream, …)
+            if(!$template->smarty->security_policy->isTrustedUri($params['file'])) {
+                return;
+            }
+        } else {
+            // local file
             if(!$template->smarty->security_policy->isTrustedResourceDir($params['file'])) {
                 return;
             }
+        }
+    }
 
-        // fetch the file
+    $content = '';
-        if($fp = @fopen($params['file'],'r')) {
+    if ($protocol == 'http') {
-            while(!feof($fp)) {
-                $content .= fgets ($fp,4096);
-            }
-            fclose($fp);
-        } else {
-            trigger_error('[plugin] fetch cannot read file \'' . $params['file'] . '\'',E_USER_NOTICE);
-            return;
-        }
-    } else {
-        // not a local file
-        if(preg_match('!^http://!i',$params['file'])) {
         // http fetch
         if($uri_parts = parse_url($params['file'])) {
             // set defaults
@@ -191,21 +198,12 @@ function smarty_function_fetch($params, $template)
             return;
         }
     } else {
-            // ftp fetch
+        $content = @file_get_contents($params['file']);
-            if($fp = @fopen($params['file'],'r')) {
+        if ($content === false) {
-                while(!feof($fp)) {
+            throw new SmartyException("{fetch} cannot read resource '" . $params['file'] ."'");
-                    $content .= fgets ($fp,4096);
-                }
-                fclose($fp);
-            } else {
-                trigger_error('[plugin] fetch cannot read file \'' . $params['file'] .'\'',E_USER_NOTICE);
-                return;
         }
     }
 
-    }
-
-
     if (!empty($params['assign'])) {
         $template->assign($params['assign'], $content);
     } else {

libs/plugins/function.html_image.php

@@ -46,8 +46,7 @@ function smarty_function_html_image($params, $template)
     $prefix = '';
     $suffix = '';
     $path_prefix = '';
-    $server_vars = $_SERVER;
+    $basedir = isset($_SERVER['DOCUMENT_ROOT']) ? $_SERVER['DOCUMENT_ROOT'] : '';
-    $basedir = isset($server_vars['DOCUMENT_ROOT']) ? $server_vars['DOCUMENT_ROOT'] : '';
     foreach($params as $_key => $_val) {
         switch ($_key) {
             case 'file':
@@ -88,13 +87,38 @@ function smarty_function_html_image($params, $template)
         return;
     } 
 
-    if (substr($file, 0, 1) == '/') {
+    if ($file[0] == '/') {
         $_image_path = $basedir . $file;
     } else {
         $_image_path = $file;
     }
     
+    // strip file protocol
+    if (stripos($params['file'], 'file://') === 0) {
+        $params['file'] = substr($params['file'], 7);
+    }
+    
+    $protocol = strpos($params['file'], '://');
+    if ($protocol !== false) {
+        $protocol = strtolower(substr($params['file'], 0, $protocol));
+    }
+    
+    if (isset($template->smarty->security_policy)) {
+        if ($protocol) {
+            // remote resource (or php stream, …)
+            if(!$template->smarty->security_policy->isTrustedUri($params['file'])) {
+                return;
+            }
+        } else {
+            // local file
+            if(!$template->smarty->security_policy->isTrustedResourceDir($params['file'])) {
+                return;
+            }
+        }
+    }
+
     if (!isset($params['width']) || !isset($params['height'])) {
+        // FIXME: (rodneyrehm) getimagesize() loads the complete file off a remote resource, use custom [jpg,png,gif]header reader!
         if (!$_image_data = @getimagesize($_image_path)) {
             if (!file_exists($_image_path)) {
                 trigger_error("html_image: unable to find '$_image_path'", E_USER_NOTICE);
@@ -107,11 +131,6 @@ function smarty_function_html_image($params, $template)
                 return;
             } 
         }
-        if (isset($template->smarty->security_policy)) {
-            if (!$template->smarty->security_policy->isTrustedResourceDir($_image_path)) {
-                return;
-            } 
-        } 
 
         if (!isset($params['width'])) {
             $width = $_image_data[0];
@@ -122,7 +141,9 @@ function smarty_function_html_image($params, $template)
     } 
 
     if (isset($params['dpi'])) {
-        if (strstr($server_vars['HTTP_USER_AGENT'], 'Mac')) {
+        if (strstr($_SERVER['HTTP_USER_AGENT'], 'Mac')) {
+            // FIXME: (rodneyrehm) wrong dpi assumption
+            // don't know who thought this up… even if it was true in 1998, it's definitely wrong in 2011.
             $dpi_default = 72;
         } else {
             $dpi_default = 96;

libs/sysplugins/smarty_security.php

@@ -48,6 +48,12 @@ class Smarty_Security {
      * @var array
      */
     public $trusted_dir = array();
+    /**
+     * List of regular expressions (PCRE) that include trusted URIs
+     *
+     * @var array
+     */
+    public $trusted_uri = array();
     /**
      * This is an array of trusted static classes.
      *
@@ -375,6 +381,32 @@ class Smarty_Security {
         throw new SmartyException("directory '{$_filepath}' not allowed by security setting");
     }
     
+    /**
+     * Check if URI (e.g. {fetch} or {html_image}) is trusted
+     *
+     * To simplify things, isTrustedUri() resolves all input to "{$PROTOCOL}://{$HOSTNAME}".
+     * So "http://username:password@hello.world.example.org:8080/some-path?some=query-string"
+     * is reduced to "http://hello.world.example.org" prior to applying the patters from {@link $trusted_uri}.
+     * @param string $uri 
+     * @return boolean true if URI is trusted
+     * @throws SmartyException if URI is not trusted
+     * @uses $trusted_uri for list of patterns to match against $uri
+     */
+    public function isTrustedUri($uri)
+    {
+        $_uri = parse_url($uri);
+        if (!empty($_uri['scheme']) && !empty($_uri['host'])) {
+            $_uri = $_uri['scheme'] . '://' . $_uri['host'];
+            foreach ($this->trusted_uri as $pattern) {
+                if (preg_match($pattern, $_uri)) {
+                    return true;
+                }
+            }
+        }
+        
+        throw new SmartyException("URI '{$uri}' not allowed by security setting");
+    }
+    
     /**
      * Check if directory of file resource is trusted.
      *