From 0460eb08cf1adf9979d0679f94bb901296e83ee6 Mon Sep 17 00:00:00 2001 From: Simon Wisselink Date: Mon, 29 Jun 2026 11:33:30 +0200 Subject: [PATCH] Security: validate nested stream wrapper in stream: resource (CWE-22) The built-in stream: resource type let a template bypass Security stream restrictions. BasePlugin::load() matches the 'stream' sysplugin before the stream_get_wrappers()/isTrustedStream() check, so a resource such as stream:php://filter/read=convert.base64-encode/resource=/path was opened by StreamPlugin::getContent() via fopen() on the nested php:// wrapper without ever validating it. This bypassed Security::$streams (including Security::$streams = null) and allowed reading arbitrary local files. Parse the wrapper scheme from the resolved path in StreamPlugin::getContent() and validate it with Security::isTrustedStream() before fopen(), giving the stream: resource the same check the direct wrapper path already receives. Adds regression tests covering the disabled-streams bypass, the not-on-allowlist case, and a positive test that an explicitly allowed wrapper still works. --- ...security-stream-resource-wrapper-bypass.md | 1 + src/Resource/StreamPlugin.php | 13 +- .../StreamWrapperSecurityTest.php | 139 ++++++++++++++++++ 3 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 changelog/security-stream-resource-wrapper-bypass.md create mode 100644 tests/UnitTests/SecurityTests/StreamWrapperSecurityTest.php diff --git a/changelog/security-stream-resource-wrapper-bypass.md b/changelog/security-stream-resource-wrapper-bypass.md new file mode 100644 index 000000000..ec7535ac0 --- /dev/null +++ b/changelog/security-stream-resource-wrapper-bypass.md @@ -0,0 +1 @@ +- Security: the built-in `stream:` resource type now validates the nested stream wrapper against the security policy, so a template such as `stream:php://filter/...` can no longer bypass `Security::$streams` (including `Security::$streams = null`) to read local files (CWE-22) diff --git a/src/Resource/StreamPlugin.php b/src/Resource/StreamPlugin.php index 9b5b3f579..9e738f046 100644 --- a/src/Resource/StreamPlugin.php +++ b/src/Resource/StreamPlugin.php @@ -54,8 +54,19 @@ public function getContent(Source $source) { $filepath = str_replace(':', '://', $source->getFullResourceName()); } + // Validate the underlying stream wrapper against the security policy. + // When the built-in "stream" resource type is used (e.g. + // stream:php://filter/...), BasePlugin::load() matches the "stream" + // sysplugin before the stream_get_wrappers()/isTrustedStream() check, + // so the nested wrapper ("php" here) is never validated. Parse the + // wrapper scheme from the resolved path and check it explicitly so that + // e.g. Security::$streams = null blocks it before fopen() (CWE-22/-441). + $smarty = $source->getSmarty(); + if (is_object($smarty->security_policy) && ($_pos = strpos($filepath, '://')) !== false) { + $smarty->security_policy->isTrustedStream(strtolower(substr($filepath, 0, $_pos))); + } + $t = ''; - // the availability of the stream has already been checked in Smarty\Resource\Base::fetch() $fp = fopen($filepath, 'r+'); if ($fp) { while (!feof($fp) && ($current_line = fgets($fp)) !== false) { diff --git a/tests/UnitTests/SecurityTests/StreamWrapperSecurityTest.php b/tests/UnitTests/SecurityTests/StreamWrapperSecurityTest.php new file mode 100644 index 000000000..eeb73e6f2 --- /dev/null +++ b/tests/UnitTests/SecurityTests/StreamWrapperSecurityTest.php @@ -0,0 +1,139 @@ +setUpSmarty(__DIR__); + $this->secretFile = sys_get_temp_dir() . DIRECTORY_SEPARATOR + . 'smarty_stream_secret_' . getmypid() . '_' . uniqid() . '.txt'; + file_put_contents($this->secretFile, 'STREAM-WRAPPER-SECRET'); + $this->smarty->setForceCompile(true); + $this->smarty->enableSecurity(); + } + + public function tearDown(): void + { + if ($this->secretFile && file_exists($this->secretFile)) { + unlink($this->secretFile); + } + parent::tearDown(); + } + + private function phpFilterUri() + { + return 'php://filter/read=convert.base64-encode/resource=' . $this->secretFile; + } + + /** + * Sanity: a direct php:// stream is rejected when all streams are disabled. + */ + public function testDirectPhpStreamIsBlocked() + { + $this->smarty->security_policy->streams = null; + $this->expectException(\Smarty\Exception::class); + $this->expectExceptionMessage("stream 'php' not allowed by security setting"); + $this->smarty->fetch('string:{include file="' . $this->phpFilterUri() . '"}'); + } + + /** + * The built-in "stream" resource type must not let a nested php:// wrapper + * escape the same restriction (CWE-22 / wrapper bypass). + */ + public function testStreamResourceCannotBypassDisabledStreams() + { + $this->smarty->security_policy->streams = null; + $this->expectException(\Smarty\Exception::class); + $this->expectExceptionMessage("stream 'php' not allowed by security setting"); + $this->smarty->fetch('string:{include file="stream:' . $this->phpFilterUri() . '"}'); + } + + /** + * Even when some streams are allowed, a nested wrapper that is not on the + * allowlist must still be rejected through the "stream" resource type. + */ + public function testStreamResourceRejectsWrapperNotOnAllowlist() + { + $this->smarty->security_policy->streams = array('file'); + $this->expectException(\Smarty\Exception::class); + $this->expectExceptionMessage("stream 'php' not allowed by security setting"); + $this->smarty->fetch('string:{include file="stream:' . $this->phpFilterUri() . '"}'); + } + + /** + * A wrapper explicitly allowed by the policy must keep working through the + * "stream" resource type (no backwards-compatibility break). + */ + public function testStreamResourceAllowsWhitelistedWrapper() + { + stream_wrapper_register('smartyteststream', 'StreamSecurityTestWrapper'); + try { + $this->smarty->security_policy->streams = array('smartyteststream'); + $this->smarty->assign('name', 'World'); + $result = $this->smarty->fetch('string:{include file="stream:smartyteststream://x"}'); + $this->assertEquals('hello World', $result); + } finally { + stream_wrapper_unregister('smartyteststream'); + } + } +} + +/** + * Minimal read-only stream wrapper returning a fixed template body, used by the + * allowlist (positive) test above. + */ +#[AllowDynamicProperties] +class StreamSecurityTestWrapper +{ + public $context; + private $pos = 0; + private $data = 'hello {$name}'; + + public function stream_open($path, $mode, $options, &$opened_path) + { + $this->pos = 0; + return true; + } + + public function stream_read($count) + { + $ret = substr($this->data, $this->pos, $count); + $this->pos += strlen($ret); + return $ret; + } + + public function stream_eof() + { + return $this->pos >= strlen($this->data); + } + + public function stream_stat() + { + return array(); + } + + public function url_stat($path, $flags) + { + return array(); + } + + public function stream_seek($offset, $whence) + { + return false; + } +}