diff --git a/src/Sandbox/MemberMatcher.php b/src/Sandbox/MemberMatcher.php new file mode 100644 index 000000000..ae9fc4c94 --- /dev/null +++ b/src/Sandbox/MemberMatcher.php @@ -0,0 +1,79 @@ + [...]` in order to allow those methods/properties for all classes. + * - Method/property can be specified as wildcard eg. `\DateTime => '*'` in order to allow all methods/properties for that class. + * - Method/property can also be specified with a trailing wildcard to allow all methods/properties with a certain prefix, eg. `\DateTime => ['get*', ...]` in order to allow all methods/properties that start with `get`. + * + * @author Yaakov Saxon + */ +final class MemberMatcher +{ + private $allowedMembers; + private $cache = []; + + public function __construct(array $allowedMembers) + { + $normalizedMembers = []; + foreach ($allowedMembers as $class => $members) { + if (!\is_array($members)) { + $normalizedMembers[$class][] = strtolower($members); + } else { + foreach ($members as $index => $member) { + $normalizedMembers[$class][$index] = strtolower($member); + } + } + } + $this->allowedMembers = $normalizedMembers; + } + + public function isAllowed($obj, string $member): bool + { + $cacheKey = get_class($obj) . "::" . $member; + + // Check cache first + if (isset($this->cache[$cacheKey])) { + return true; + } + + $member = strtolower($member); // normalize member name + + foreach ($this->allowedMembers as $class => $members) { + if ('*' === $class || $obj instanceof $class) { + foreach ($members as $allowedMember) { + if ('*' === $allowedMember) { + $this->cache[$cacheKey] = true; + + return true; + } + if ($allowedMember === $member) { + $this->cache[$cacheKey] = true; + + return true; + } + // if allowedMember ends with a *, check if the member starts with the allowedMember + if ('*' === substr($allowedMember, -1) && substr($member, 0, \strlen($allowedMember) - 1) === rtrim($allowedMember, '*')) { + $this->cache[$cacheKey] = true; + + return true; + } + } + } + } + + // If we reach here, the member is not allowed + return false; + } +} diff --git a/src/Sandbox/SecurityPolicy.php b/src/Sandbox/SecurityPolicy.php index 3b79a870f..b62ce47c4 100644 --- a/src/Sandbox/SecurityPolicy.php +++ b/src/Sandbox/SecurityPolicy.php @@ -32,7 +32,7 @@ public function __construct(array $allowedTags = [], array $allowedFilters = [], $this->allowedTags = $allowedTags; $this->allowedFilters = $allowedFilters; $this->setAllowedMethods($allowedMethods); - $this->allowedProperties = $allowedProperties; + $this->setAllowedProperties($allowedProperties); $this->allowedFunctions = $allowedFunctions; } @@ -48,15 +48,12 @@ public function setAllowedFilters(array $filters) public function setAllowedMethods(array $methods) { - $this->allowedMethods = []; - foreach ($methods as $class => $m) { - $this->allowedMethods[$class] = array_map(function ($value) { return strtr($value, 'ABCDEFGHIJKLMNOPQRSTUVWXYZ', 'abcdefghijklmnopqrstuvwxyz'); }, \is_array($m) ? $m : [$m]); - } + $this->allowedMethods = new MemberMatcher($methods); } public function setAllowedProperties(array $properties) { - $this->allowedProperties = $properties; + $this->allowedProperties = new MemberMatcher($properties); } public function setAllowedFunctions(array $functions) @@ -91,32 +88,16 @@ public function checkMethodAllowed($obj, $method) return; } - $allowed = false; - $method = strtr($method, 'ABCDEFGHIJKLMNOPQRSTUVWXYZ', 'abcdefghijklmnopqrstuvwxyz'); - foreach ($this->allowedMethods as $class => $methods) { - if ($obj instanceof $class && \in_array($method, $methods)) { - $allowed = true; - break; - } - } - - if (!$allowed) { + if (!$this->allowedMethods->isAllowed($obj, $method)) { $class = \get_class($obj); + $method = strtr($method, 'ABCDEFGHIJKLMNOPQRSTUVWXYZ', 'abcdefghijklmnopqrstuvwxyz'); throw new SecurityNotAllowedMethodError(sprintf('Calling "%s" method on a "%s" object is not allowed.', $method, $class), $class, $method); } } public function checkPropertyAllowed($obj, $property) { - $allowed = false; - foreach ($this->allowedProperties as $class => $properties) { - if ($obj instanceof $class && \in_array($property, \is_array($properties) ? $properties : [$properties])) { - $allowed = true; - break; - } - } - - if (!$allowed) { + if (!$this->allowedProperties->isAllowed($obj, $property)) { $class = \get_class($obj); throw new SecurityNotAllowedPropertyError(sprintf('Calling "%s" property on a "%s" object is not allowed.', $property, $class), $class, $property); } diff --git a/tests/Extension/SandboxTest.php b/tests/Extension/SandboxTest.php index dff2ddc8d..8fbf06284 100644 --- a/tests/Extension/SandboxTest.php +++ b/tests/Extension/SandboxTest.php @@ -449,6 +449,87 @@ public function testMultipleClassMatchesViaInheritanceInAllowedMethods() } } + public function testSandboxAllowedWildcardMethod() + { + $allowedMethods = ['Twig\Tests\Extension\FooObject' => ['*']]; + $twig = $this->getEnvironment(true, [], self::$templates, [], [], $allowedMethods); + + // This should pass without throwing an exception + $result = $twig->load('1_basic1')->render(self::$params); + $this->assertEquals('foo', $result); + $result = $twig->load('1_basic8')->render(self::$params); + $this->assertEquals('foobarfoobar', $result); + + } + + public function testSandboxAllowedWildcardSuffixMethod() + { + $allowedMethods = ['Twig\Tests\Extension\FooObject' => ['get*']]; + $twig = $this->getEnvironment(true, [], self::$templates, [], [], $allowedMethods); + + // This should pass without throwing an exception + $result = $twig->load('1_basic8')->render(self::$params); + $this->assertEquals('foobarfoobar', $result); + } + + public function testSandboxUnallowedPrefixWildcardSuffixMethod() + { + $allowedMethods = ['Twig\Tests\Extension\FooObject' => ['get*']]; + $twig = $this->getEnvironment(true, [], self::$templates, [], [], $allowedMethods); + + try { + $twig->load('1_basic1')->render(self::$params); + $this->fail('Sandbox should block method not matching method prefix'); + } catch (SecurityError $e) { + $this->assertInstanceOf(SecurityNotAllowedMethodError::class, $e); + $this->assertEquals('foo', $e->getMethodName()); + } + } + + public function testSandboxAllowedWildcardClassMethod() + { + $allowedMethods = ['*' => ['foo']]; + $twig = $this->getEnvironment(true, [], self::$templates, [], [], $allowedMethods); + + // This should pass without throwing an exception + $result = $twig->load('1_basic1')->render(self::$params); + $this->assertEquals('foo', $result); + } + + public function testSandboxUnallowedWildcardClassMethod() + { + $allowedMethods = ['*' => ['foo']]; + $twig = $this->getEnvironment(true, [], self::$templates, [], [], $allowedMethods); + + try { + $twig->load('1_basic8')->render(self::$params); + $this->fail('Sandbox should block non matching method despite class wildcard'); + } catch (SecurityError $e) { + $this->assertInstanceOf(SecurityNotAllowedMethodError::class, $e); + $this->assertEquals('getfoobar', $e->getMethodName()); + } + } + + public function testSandboxAllowedWildcardClassWildcardSuffixMethod() + { + $allowedMethods = ['*' => ['get*']]; + $twig = $this->getEnvironment(true, [], self::$templates, [], [], $allowedMethods); + + // This should pass without throwing an exception + $result = $twig->load('1_basic8')->render(self::$params); + $this->assertEquals('foobarfoobar', $result); + } + + public function testSandboxAllowedWildcardClassWildcardSuffixProperty() + { + $allowedProperties = ['*' => ['b*']]; + $twig = $this->getEnvironment(true, [], self::$templates, [], [], [], $allowedProperties); + + // This should pass without throwing an exception + $result = $twig->load('1_basic4')->render(self::$params); + $this->assertEquals('bar', $result); + } + protected function getEnvironment($sandboxed, $options, $templates, $tags = [], $filters = [], $methods = [], $properties = [], $functions = []) { $loader = new ArrayLoader($templates);