Skip to content

Commit

Permalink
Merge branch '3.14.x' into 3.x
Browse files Browse the repository at this point in the history
* 3.14.x:
  Improve detection of recursion
  Fix recursion when arrays contain self-references in sandboxed mode
  Fix code
  Prepare the 3.11.2 release
  Update CHANGELOG
  Sandbox ArrayAccess and do sandbox checks before isset() checks
  Fix sandbox handling for __toString()
  Prepare the 3.14.1 release
  Update CHANGELOG
  Sandbox ArrayAccess and do sandbox checks before isset() checks
  Fix sandbox handling for __toString()
  Prepare the 3.11.1 release
  Fix a security issue when an included sandboxed template has been loaded before without the sandbox context
  • Loading branch information
nicolas-grekas committed Nov 7, 2024
2 parents 32e99c2 + 83a21d3 commit d72ae24
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 15 deletions.
18 changes: 18 additions & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@
* Add the `enum` function
* Add support for logical `xor` operator

# 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
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.14.0 (2024-09-09)

* Fix a security issue when an included sandboxed template has been loaded before without the sandbox context
Expand Down Expand Up @@ -87,6 +94,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`
Expand Down
2 changes: 1 addition & 1 deletion doc/sandbox.rst
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,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.

Expand Down
8 changes: 2 additions & 6 deletions src/Extension/CoreExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,6 @@

final class CoreExtension extends AbstractExtension
{
private const DEFAULT_TRIM_CHARS = " \t\n\r\0\x0B";

public const ARRAY_LIKE_CLASSES = [
'ArrayIterator',
'ArrayObject',
Expand All @@ -114,6 +112,8 @@ final class CoreExtension extends AbstractExtension
'WeakMap',
];

private const DEFAULT_TRIM_CHARS = " \t\n\r\0\x0B";

private $dateFormats = ['F j, Y H:i', '%d days'];
private $numberFormat = [0, '.', ','];
private $timezone = null;
Expand Down Expand Up @@ -1744,10 +1744,6 @@ public static function getAttribute(Environment $env, Source $source, $object, $
return true;
}

if ($sandboxed) {
$env->getExtension(SandboxExtension::class)->checkPropertyAllowed($object, $item, $lineno, $source);
}

return \constant($object::class.'::'.$item);
}
}
Expand Down
30 changes: 26 additions & 4 deletions src/Extension/SandboxExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
}
}
}
12 changes: 8 additions & 4 deletions tests/Extension/SandboxTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}',
Expand Down Expand Up @@ -317,6 +320,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(", ") }}'],
];
}

Expand Down Expand Up @@ -650,12 +654,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
Expand All @@ -671,11 +675,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.');
}
}

0 comments on commit d72ae24

Please sign in to comment.