Port security fixes from v5.8.2 to Smarty v4.

changelog/security-fetch-ssrf-redirect.md

@@ -0,0 +1 @@
+- Security: `{fetch}` no longer follows HTTP redirects for remote resources while a security policy is active, preventing an open redirect on a trusted host from bypassing `trusted_uri` (CWE-918 server-side request forgery)

changelog/security-html-image-escaping.md

@@ -0,0 +1 @@
+- Security: `{html_image}` now escapes the `file`, `path_prefix`, `href`/`link`, `width` and `height` attributes (it already escaped `alt` and pass-through attributes), and `{html_select_date}` casts `day_size`/`month_size`/`year_size` to int (matching `{html_select_time}`), preventing untrusted values passed into these attributes from breaking out of the generated HTML (CWE-79)

changelog/security-symlink-path-traversal.md

@@ -0,0 +1 @@
+- Security: prevent symlinks inside a trusted `secure_dir`/template directory from being used to read files outside of it (CWE-22 path traversal), affecting `{include}` and `{fetch}` of local files

libs/plugins/function.fetch.php

@@ -191,7 +191,24 @@ function smarty_function_fetch($params, $template)
             return;
         }
     } else {
-        $content = @file_get_contents($params[ 'file' ]);
+        if ($protocol && isset($template->smarty->security_policy)) {
+            // Remote resource (e.g. https://) reached through file_get_contents().
+            // isTrustedUri() only validates the initial URL, but file_get_contents()
+            // follows redirects by default, so an open redirect on an otherwise
+            // trusted host could be used to reach a non-trusted target (SSRF).
+            // Disable redirect-following while a security policy is in effect.
+            $context = stream_context_create(
+                array(
+                    'http' => array(
+                        'follow_location' => 0,
+                        'max_redirects'   => 1,
+                    ),
+                )
+            );
+            $content = @file_get_contents($params[ 'file' ], false, $context);
+        } else {
+            $content = @file_get_contents($params[ 'file' ]);
+        }
         if ($content === false) {
             throw new SmartyException("{fetch} cannot read resource '" . $params[ 'file' ] . "'");
         }

libs/plugins/function.html_image.php

@@ -75,7 +75,7 @@ function smarty_function_html_image($params, Smarty_Internal_Template $template)
                 break;
             case 'link':
             case 'href':
-                $prefix = '<a href="' . $_val . '">';
+                $prefix = '<a href="' . smarty_function_escape_special_chars($_val) . '">';
                 $suffix = '</a>';
                 break;
             default:
@@ -153,6 +153,11 @@ function smarty_function_html_image($params, Smarty_Internal_Template $template)
         $width = round($width * $_resize);
         $height = round($height * $_resize);
     }
-    return $prefix . '<img src="' . $path_prefix . $file . '" alt="' . $alt . '" width="' . $width . '" height="' .
-           $height . '"' . $extra . ' />' . $suffix;
+    // $alt and the pass-through attributes ($extra) are already escaped above;
+    // escape the remaining value-context params at output time so untrusted
+    // values cannot break out of the attribute (CWE-79). The unescaped $file/
+    // $width/$height are still used for getimagesize()/DPI math above.
+    return $prefix . '<img src="' . smarty_function_escape_special_chars($path_prefix . $file) . '" alt="' . $alt
+           . '" width="' . smarty_function_escape_special_chars($width) . '" height="'
+           . smarty_function_escape_special_chars($height) . '"' . $extra . ' />' . $suffix;
 }

libs/plugins/function.html_select_date.php

@@ -131,9 +131,6 @@ function smarty_function_html_select_date($params, Smarty_Internal_Template $tem
             case 'day_value_format':
             case 'month_format':
             case 'month_value_format':
-            case 'day_size':
-            case 'month_size':
-            case 'year_size':
             case 'all_extra':
             case 'day_extra':
             case 'month_extra':
@@ -151,6 +148,13 @@ function smarty_function_html_select_date($params, Smarty_Internal_Template $tem
             case 'year_id':
                 $$_key = (string)$_value;
                 break;
+            case 'day_size':
+            case 'month_size':
+            case 'year_size':
+                // numeric HTML size attribute; cast to int (consistent with
+                // html_select_time) so it cannot break out of size="…" (CWE-79)
+                $$_key = (int)$_value;
+                break;
             case 'display_days':
             case 'display_months':
             case 'display_years':

libs/sysplugins/smarty_security.php

@@ -590,12 +590,34 @@ private function _updateResourceDir($oldDir, $newDir)
      */
     private function _checkDir($filepath, $dirs)
     {
-        $directory = dirname($this->smarty->_realpath($filepath, true)) . DIRECTORY_SEPARATOR;
+        // Resolve the canonical, symlink-free path of the requested file so that
+        // a symlink located inside a trusted directory cannot be abused to read
+        // a file outside of it (CWE-22 path traversal). Smarty::_realpath() only
+        // normalizes the path as a string and does not follow symlinks, so we
+        // fall back to it only when the file does not yet exist on disk (e.g.
+        // config/cache paths that are validated before being written).
+        $realpath = @realpath($filepath);
+        $resolved = $realpath !== false ? $realpath : $this->smarty->_realpath($filepath, true);
+        $directory = dirname($resolved) . DIRECTORY_SEPARATOR;
+
+        // Canonicalize the trusted directories the same way. This keeps
+        // legitimate symlinked deployment paths working (e.g. a Capistrano-style
+        // "current" release symlink, or macOS' /var -> /private/var): both the
+        // file and the trusted directories are compared after symlinks have been
+        // resolved.
+        $trusted = array();
+        foreach ($dirs as $dir => $unused) {
+            $trusted[ $dir ] = true;
+            if (($dirRealpath = @realpath($dir)) !== false) {
+                $trusted[ rtrim($dirRealpath, '\\/') . DIRECTORY_SEPARATOR ] = true;
+            }
+        }
+
         $_directory = array();
         if (!preg_match('#[\\\\/][.][.][\\\\/]#', $directory)) {
             while (true) {
                 // test if the directory is trusted
-                if (isset($dirs[ $directory ])) {
+                if (isset($trusted[ $directory ])) {
                     return $_directory;
                 }
                 // abort if we've reached root

tests/UnitTests/SecurityTests/SecurityTest.php

@@ -375,6 +375,87 @@ public function testSmartyTemplateObject() {
         $this->smarty->display('string:{$smarty.template_object}');
     }
 
+    /**
+     * A symlink located inside a trusted secure_dir must not be usable to read
+     * a file outside of it (CWE-22 path traversal via symlink).
+     */
+    public function testSymlinkEscapeFromSecureDirIsRejected()
+    {
+        list($secureDir, $outsideFile) = $this->createSymlinkFixture('secret-outside-content');
+        $link = $secureDir . DIRECTORY_SEPARATOR . 'finance_doc';
+        if (!@symlink($outsideFile, $link)) {
+            $this->markTestSkipped('Unable to create symlinks on this platform');
+        }
+
+        $this->smarty->security_policy->secure_dir = array($secureDir . DIRECTORY_SEPARATOR);
+
+        $this->expectException('SmartyException');
+        $this->expectExceptionMessage('not trusted file path');
+        // Use forward slashes: backslashes in a double-quoted template string are
+        // interpreted as escape sequences (\f, \r, ...), which would corrupt a
+        // Windows path. Forward slashes work on every platform.
+        $this->smarty->fetch('string:{include file="' . str_replace('\\', '/', $link) . '"}');
+    }
+
+    /**
+     * A symlink that stays inside the trusted secure_dir must keep working, so
+     * legitimate (e.g. deployment) symlinks are not broken by the fix above.
+     */
+    public function testSymlinkWithinSecureDirIsAllowed()
+    {
+        list($secureDir) = $this->createSymlinkFixture('secret-outside-content');
+        $target = $secureDir . DIRECTORY_SEPARATOR . 'real.tpl';
+        file_put_contents($target, 'inside-content');
+        $link = $secureDir . DIRECTORY_SEPARATOR . 'linked.tpl';
+        if (!@symlink($target, $link)) {
+            $this->markTestSkipped('Unable to create symlinks on this platform');
+        }
+
+        $this->smarty->security_policy->secure_dir = array($secureDir . DIRECTORY_SEPARATOR);
+
+        // Forward slashes so backslashes in a Windows path are not mistaken for
+        // escape sequences inside the double-quoted template string.
+        $this->assertEquals('inside-content', $this->smarty->fetch('string:{include file="' . str_replace('\\', '/', $link) . '"}'));
+    }
+
+    /**
+     * Builds a temporary directory tree for the symlink tests: a (canonicalized)
+     * secure directory plus a file located outside of it. The tree is removed in
+     * tearDown(). Returns array(secureDir, outsideFile).
+     */
+    private function createSymlinkFixture($outsideContent)
+    {
+        $base = sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'smarty_symlink_' . getmypid() . '_' . uniqid();
+        $secureDir = $base . DIRECTORY_SEPARATOR . 'secure';
+        mkdir($secureDir, 0777, true);
+        $outsideFile = $base . DIRECTORY_SEPARATOR . 'outside.txt';
+        file_put_contents($outsideFile, $outsideContent);
+
+        // Canonicalize so secure_dir is symlink-free (sys_get_temp_dir() itself
+        // may sit under a symlink, e.g. /var -> /private/var on macOS).
+        $this->symlinkFixtureDir = realpath($base);
+        return array(realpath($secureDir), realpath($outsideFile));
+    }
+
+    /** @var string|null temp dir created by createSymlinkFixture(), removed in tearDown */
+    private $symlinkFixtureDir = null;
+
+    protected function tearDown(): void
+    {
+        if (!empty($this->symlinkFixtureDir) && is_dir($this->symlinkFixtureDir)) {
+            $it = new \RecursiveIteratorIterator(
+                new \RecursiveDirectoryIterator($this->symlinkFixtureDir, \FilesystemIterator::SKIP_DOTS),
+                \RecursiveIteratorIterator::CHILD_FIRST
+            );
+            foreach ($it as $entry) {
+                ($entry->isDir() && !$entry->isLink()) ? rmdir($entry->getPathname()) : unlink($entry->getPathname());
+            }
+            rmdir($this->symlinkFixtureDir);
+            $this->symlinkFixtureDir = null;
+        }
+        parent::tearDown();
+    }
+
 }
 
 class mysecuritystaticclass

tests/UnitTests/TemplateSource/TagTests/PluginFunction/PluginFunctionFetchTest.php

@@ -82,4 +82,99 @@ public function testFetchSecurity2()
       $this->smarty->fetch('string:{fetch file="/templates/../etc/passwd"}');
   }
 
+    /**
+     * When a security policy is in effect, {fetch} of a remote resource must not
+     * follow redirects, otherwise an open redirect on a trusted host could be
+     * used to bypass trusted_uri and reach an internal target (SSRF, CWE-918).
+     *
+     * @runInSeparateProcess
+     * @preserveGlobalState disabled
+     */
+    public function testFetchRemoteDisablesRedirectsUnderSecurity()
+    {
+        FetchContextCaptureStreamWrapper::$capturedOptions = null;
+        stream_wrapper_register('ssrftest', 'FetchContextCaptureStreamWrapper');
+        try {
+            $this->smarty->enableSecurity();
+            $this->smarty->security_policy->trusted_uri[] = '/^ssrftest:\/\/allowed$/';
+
+            $result = $this->smarty->fetch('string:{fetch file="ssrftest://allowed/data"}');
+
+            $this->assertSame('BODY', $result);
+            $this->assertIsArray(FetchContextCaptureStreamWrapper::$capturedOptions);
+            $this->assertArrayHasKey('http', FetchContextCaptureStreamWrapper::$capturedOptions);
+            $this->assertSame(0, FetchContextCaptureStreamWrapper::$capturedOptions['http']['follow_location']);
+            $this->assertLessThanOrEqual(1, FetchContextCaptureStreamWrapper::$capturedOptions['http']['max_redirects']);
+        } finally {
+            stream_wrapper_unregister('ssrftest');
+        }
+    }
+
+    /**
+     * Without a security policy there is no trusted_uri to bypass, so the
+     * redirect-disabling stream context is not applied (backwards compatible).
+     *
+     * @runInSeparateProcess
+     * @preserveGlobalState disabled
+     */
+    public function testFetchRemoteKeepsDefaultBehaviorWithoutSecurity()
+    {
+        FetchContextCaptureStreamWrapper::$capturedOptions = null;
+        stream_wrapper_register('ssrftest', 'FetchContextCaptureStreamWrapper');
+        try {
+            $result = $this->smarty->fetch('string:{fetch file="ssrftest://allowed/data"}');
+
+            $this->assertSame('BODY', $result);
+            $this->assertSame(array(), FetchContextCaptureStreamWrapper::$capturedOptions);
+        } finally {
+            stream_wrapper_unregister('ssrftest');
+        }
+    }
+
+}
+
+/**
+ * Minimal custom stream wrapper used by the fetch SSRF tests: it records the
+ * stream context options that {fetch} passes to file_get_contents() and returns
+ * a fixed body so the call succeeds without touching the network.
+ */
+class FetchContextCaptureStreamWrapper
+{
+    /** @var resource|null populated by PHP when a context is passed */
+    public $context;
+
+    /** @var array|null options captured from the context on the last open */
+    public static $capturedOptions = null;
+
+    private $read = false;
+
+    public function stream_open($path, $mode, $options, &$opened_path)
+    {
+        self::$capturedOptions = isset($this->context) ? stream_context_get_options($this->context) : array();
+        return true;
+    }
+
+    public function stream_read($count)
+    {
+        if ($this->read) {
+            return '';
+        }
+        $this->read = true;
+        return 'BODY';
+    }
+
+    public function stream_eof()
+    {
+        return $this->read;
+    }
+
+    public function stream_stat()
+    {
+        return array();
+    }
+
+    public function url_stat($path, $flags)
+    {
+        return array();
+    }
 }

tests/UnitTests/TemplateSource/TagTests/PluginFunction/PluginFunctionHtmlImageTest.php

@@ -0,0 +1,84 @@
+<?php
+/**
+ * Smarty PHPunit tests of the {html_image} function plugin
+ *
+ * @package PHPunit
+ */
+
+/**
+ * class for {html_image} tests
+ *
+ * @runTestsInSeparateProcess
+ * @preserveGlobalState disabled
+ * @backupStaticAttributes enabled
+ */
+class PluginFunctionHtmlImageTest extends PHPUnit_Smarty
+{
+    public function setUp(): void
+    {
+        $this->setUpSmarty(__DIR__);
+    }
+
+    public function testInit()
+    {
+        $this->cleanDirs();
+    }
+
+    /**
+     * Passing both width and height skips the getimagesize() lookup, so no real
+     * image file is needed to render the tag.
+     */
+    private function render($params)
+    {
+        $tpl = $this->smarty->createTemplate('eval:{html_image file=$file width=$width height=$height href=$href path_prefix=$path_prefix}');
+        $tpl->assign($params + array(
+            'file'        => 'pic.jpg',
+            'width'       => 44,
+            'height'      => 68,
+            'href'        => '',
+            'path_prefix' => '',
+        ));
+        return $tpl->fetch();
+    }
+
+    public function testHrefIsEscaped()
+    {
+        $result = $this->render(array('href' => '"><script>alert(1)</script>'));
+        $this->assertStringNotContainsString('<script>', $result);
+        $this->assertStringContainsString('&lt;script&gt;', $result);
+    }
+
+    public function testWidthIsEscaped()
+    {
+        $result = $this->render(array('width' => '44" onload="alert(1)'));
+        $this->assertStringNotContainsString('onload="', $result);
+        $this->assertStringContainsString('&quot;', $result);
+    }
+
+    public function testHeightIsEscaped()
+    {
+        $result = $this->render(array('height' => '68" onmouseover="alert(1)'));
+        $this->assertStringNotContainsString('onmouseover="', $result);
+    }
+
+    public function testFileAndPathPrefixAreEscaped()
+    {
+        $result = $this->render(array('file' => 'pic.jpg"><script>alert(1)</script>', 'path_prefix' => '"><b>'));
+        $this->assertStringNotContainsString('<script>', $result);
+        $this->assertStringNotContainsString('<b>', $result);
+    }
+
+    /**
+     * Benign values must be unchanged (no breakage, no double-encoding of an
+     * ampersand already present in a URL).
+     */
+    public function testBenignValuesAreUnchanged()
+    {
+        $result = $this->render(array('width' => 44, 'height' => 68, 'href' => 'detail.php?id=1&page=2'));
+        $this->assertStringContainsString('width="44"', $result);
+        $this->assertStringContainsString('height="68"', $result);
+        $this->assertStringContainsString('src="pic.jpg"', $result);
+        $this->assertStringContainsString('href="detail.php?id=1&amp;page=2"', $result);
+        $this->assertStringNotContainsString('&amp;amp;', $result);
+    }
+}