diff --git a/CHANGELOG b/CHANGELOG index 2068f4899bb..3c88b9a51b6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,6 @@ # 3.14.1 (2024-11-06) - * [BC BREAK] Fix a security issue in the sandbox mode allowing an attacker to call attributes on Array-like objects + * [BC BREAK] Fix a security issue in the sandbox mode allowing an attacker to call attributes on Array-like objects They are now checked via the property policy * Fix a security issue in the sandbox mode allowing an attacker to be able to call `toString()` under some circumstances on an object even if the `__toString()` method is not allowed by the security policy @@ -51,6 +51,17 @@ * Fix integration tests when a test has more than one data/expect section and deprecations * Add the `enum_cases` function +# 3.11.2 (2024-11-06) + + * [BC BREAK] Fix a security issue in the sandbox mode allowing an attacker to call attributes on Array-like objects + They are now checked via the property policy + * Fix a security issue in the sandbox mode allowing an attacker to be able to call `toString()` + under some circumstances on an object even if the `__toString()` method is not allowed by the security policy + +# 3.11.1 (2024-09-10) + + * Fix a security issue when an included sandboxed template has been loaded before without the sandbox context + # 3.11.0 (2024-08-08) * Deprecate `OptimizerNodeVisitor::OPTIMIZE_RAW_FILTER` diff --git a/doc/api.rst b/doc/api.rst index d5e484aa87b..4364c7bfb53 100644 --- a/doc/api.rst +++ b/doc/api.rst @@ -488,7 +488,7 @@ won't be allowed and will generate a ``\Twig\Sandbox\SecurityError`` exception. .. note:: - As of Twig 1.14.1 (and on Twig 3.11.2), if the ``Article`` class implements + As of Twig 3.14.1 (and on Twig 3.11.2), if the ``Article`` class implements the ``ArrayAccess`` interface, the templates will only be able to access the ``title`` and ``body`` attributes. diff --git a/src/Extension/SandboxExtension.php b/src/Extension/SandboxExtension.php index c9ffe6477bd..bbf73964574 100644 --- a/src/Extension/SandboxExtension.php +++ b/src/Extension/SandboxExtension.php @@ -120,14 +120,12 @@ public function checkPropertyAllowed($obj, $property, int $lineno = -1, ?Source public function ensureToStringAllowed($obj, int $lineno = -1, ?Source $source = null) { if (\is_array($obj)) { - foreach ($obj as $v) { - $this->ensureToStringAllowed($v, $lineno, $source); - } + $this->ensureToStringAllowedForArray($obj, $lineno, $source); return $obj; } - if ($this->isSandboxed($source) && $obj instanceof \Stringable) { + if ($obj instanceof \Stringable && $this->isSandboxed($source)) { try { $this->policy->checkMethodAllowed($obj, '__toString'); } catch (SecurityNotAllowedMethodError $e) { @@ -140,4 +138,28 @@ public function ensureToStringAllowed($obj, int $lineno = -1, ?Source $source = return $obj; } + + private function ensureToStringAllowedForArray(array $obj, int $lineno, ?Source $source, array &$stack = []): void + { + foreach ($obj as $k => $v) { + if (!$v) { + continue; + } + + if (!\is_array($v)) { + $this->ensureToStringAllowed($v, $lineno, $source); + continue; + } + + if ($r = \ReflectionReference::fromArrayElement($obj, $k)) { + if (isset($stack[$r->getId()])) { + continue; + } + + $stack[$r->getId()] = true; + } + + $this->ensureToStringAllowedForArray($v, $lineno, $source, $stack); + } + } } diff --git a/tests/Extension/SandboxTest.php b/tests/Extension/SandboxTest.php index 999483241ae..3f8d9c952d3 100644 --- a/tests/Extension/SandboxTest.php +++ b/tests/Extension/SandboxTest.php @@ -45,7 +45,10 @@ protected function setUp(): void 'some_array' => [5, 6, 7, new FooObject()], 'array_like' => new ArrayLikeObject(), 'magic' => new MagicObject(), + 'recursion' => [4], ]; + self::$params['recursion'][] = &self::$params['recursion']; + self::$params['recursion'][] = new FooObject(); self::$templates = [ '1_basic1' => '{{ obj.foo }}', @@ -302,6 +305,7 @@ public static function getSandboxUnallowedToStringTests() 'context' => ['{{ _context|join(", ") }}'], 'spread_array_operator' => ['{{ [1, 2, ...[5, 6, 7, obj]]|join(",") }}'], 'spread_array_operator_var' => ['{{ [1, 2, ...some_array]|join(",") }}'], + 'recursion' => ['{{ recursion|join(", ") }}'], ]; } @@ -635,12 +639,12 @@ class ArrayLikeObject extends \ArrayObject { public function offsetExists($offset): bool { - throw new \BadMethodCallException('Should not be called'); + throw new \BadMethodCallException('Should not be called.'); } public function offsetGet($offset): mixed { - throw new \BadMethodCallException('Should not be called'); + throw new \BadMethodCallException('Should not be called.'); } public function offsetSet($offset, $value): void @@ -656,11 +660,11 @@ class MagicObject { public function __get($name): mixed { - throw new \BadMethodCallException('Should not be called'); + throw new \BadMethodCallException('Should not be called.'); } public function __isset($name): bool { - throw new \BadMethodCallException('Should not be called'); + throw new \BadMethodCallException('Should not be called.'); } }