From 670dae0985322d5e315dba047913f907189aa0c8 Mon Sep 17 00:00:00 2001 From: Divesh Pahuja Date: Tue, 11 Oct 2022 09:46:44 +0200 Subject: [PATCH 01/14] [Mail] Renderer email content twig templates in a sandbox --- lib/Mail.php | 8 ++++-- .../TwigDefaultDelegatingEngine.php | 27 ++++++++++++++++--- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/lib/Mail.php b/lib/Mail.php index 561a8814010..849ab033779 100644 --- a/lib/Mail.php +++ b/lib/Mail.php @@ -654,10 +654,14 @@ private function getDebugMailRecipients(array $recipients): array private function renderParams(string $string): string { $templatingEngine = \Pimcore::getContainer()->get('pimcore.templating.engine.delegating'); - $twig = $templatingEngine->getTwigEnvironment(); + $twig = $templatingEngine->getTwigEnvironment(true); $template = $twig->createTemplate($string); - return $template->render($this->getParams()); + $rendered = $template->render($this->getParams()); + + $templatingEngine->resetSandboxExtensionFromTwigEnvironment(); + + return $rendered; } /** diff --git a/lib/Templating/TwigDefaultDelegatingEngine.php b/lib/Templating/TwigDefaultDelegatingEngine.php index a313ac6245d..1221cc73077 100644 --- a/lib/Templating/TwigDefaultDelegatingEngine.php +++ b/lib/Templating/TwigDefaultDelegatingEngine.php @@ -19,6 +19,8 @@ use Symfony\Component\Templating\DelegatingEngine as BaseDelegatingEngine; use Symfony\Component\Templating\EngineInterface; use Twig\Environment; +use Twig\Extension\SandboxExtension; +use Twig\Sandbox\SecurityPolicy; /** * @internal @@ -100,14 +102,31 @@ public function isDelegate() return $this->delegate; } - /** - * @return Environment - */ - public function getTwigEnvironment(): Environment + public function getTwigEnvironment($sandboxed = false): Environment { + if ($sandboxed && !$this->twig->hasExtension(SandboxExtension::class)) { + $tags = ['if', 'include', 'import', 'block', 'set', 'for']; + $filters = ['date', 'escape', 'trans', 'split', 'length', 'slice', 'lower', 'raw']; + $methods = $properties = []; + $functions = ['include', 'path', 'absolute_url', 'asset', 'is_granted']; + + $policy = new SecurityPolicy($tags, $filters, $methods, $properties, $functions); + $sandbox = new SandboxExtension($policy); + $this->twig->addExtension($sandbox); + } + return $this->twig; } + public function resetSandboxExtensionFromTwigEnvironment(): void + { + $extensions = $this->twig->getExtensions(); + + unset($extensions[SandboxExtension::class]); + + $this->twig->setExtensions($extensions); + } + /** * @param string $view * @param array $parameters From 08382e1943c6560bf1e0f6224dde4ea0017ca71b Mon Sep 17 00:00:00 2001 From: Divesh Pahuja Date: Tue, 11 Oct 2022 10:53:03 +0200 Subject: [PATCH 02/14] [Mail] Renderer email content twig templates in a sandbox --- lib/Mail.php | 2 +- .../TwigDefaultDelegatingEngine.php | 34 +++++++++++-------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/lib/Mail.php b/lib/Mail.php index 849ab033779..bb8aeb6dd38 100644 --- a/lib/Mail.php +++ b/lib/Mail.php @@ -659,7 +659,7 @@ private function renderParams(string $string): string $rendered = $template->render($this->getParams()); - $templatingEngine->resetSandboxExtensionFromTwigEnvironment(); + $templatingEngine->disableSandboxExtensionFromTwigEnvironment(); return $rendered; } diff --git a/lib/Templating/TwigDefaultDelegatingEngine.php b/lib/Templating/TwigDefaultDelegatingEngine.php index 1221cc73077..bab8594439d 100644 --- a/lib/Templating/TwigDefaultDelegatingEngine.php +++ b/lib/Templating/TwigDefaultDelegatingEngine.php @@ -104,27 +104,31 @@ public function isDelegate() public function getTwigEnvironment($sandboxed = false): Environment { - if ($sandboxed && !$this->twig->hasExtension(SandboxExtension::class)) { - $tags = ['if', 'include', 'import', 'block', 'set', 'for']; - $filters = ['date', 'escape', 'trans', 'split', 'length', 'slice', 'lower', 'raw']; - $methods = $properties = []; - $functions = ['include', 'path', 'absolute_url', 'asset', 'is_granted']; - - $policy = new SecurityPolicy($tags, $filters, $methods, $properties, $functions); - $sandbox = new SandboxExtension($policy); - $this->twig->addExtension($sandbox); + if ($sandboxed) { + if (!$this->twig->hasExtension(SandboxExtension::class)) { + $tags = ['if', 'include', 'import', 'block', 'set', 'for']; + $filters = ['date', 'escape', 'trans', 'split', 'length', 'slice', 'lower', 'raw']; + $methods = $properties = []; + $functions = ['include', 'path', 'absolute_url', 'asset', 'is_granted']; + + $policy = new SecurityPolicy($tags, $filters, $methods, $properties, $functions); + $sandbox = new SandboxExtension($policy); + $this->twig->addExtension($sandbox); + } + + $sandbox = $this->twig->getExtension(SandboxExtension::class); + $sandbox->enableSandbox(); } return $this->twig; } - public function resetSandboxExtensionFromTwigEnvironment(): void + public function disableSandboxExtensionFromTwigEnvironment(): void { - $extensions = $this->twig->getExtensions(); - - unset($extensions[SandboxExtension::class]); - - $this->twig->setExtensions($extensions); + if ($this->twig->hasExtension(SandboxExtension::class)) { + $sandbox = $this->twig->getExtension(SandboxExtension::class); + $sandbox->disableSandbox(); + } } /** From 378073c0466464ae75dbda486608416798a99e83 Mon Sep 17 00:00:00 2001 From: Divesh Pahuja Date: Tue, 11 Oct 2022 11:08:34 +0200 Subject: [PATCH 03/14] [Mail] Renderer email content twig templates in a sandbox --- lib/Templating/TwigDefaultDelegatingEngine.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/Templating/TwigDefaultDelegatingEngine.php b/lib/Templating/TwigDefaultDelegatingEngine.php index bab8594439d..193a6c063d1 100644 --- a/lib/Templating/TwigDefaultDelegatingEngine.php +++ b/lib/Templating/TwigDefaultDelegatingEngine.php @@ -116,6 +116,7 @@ public function getTwigEnvironment($sandboxed = false): Environment $this->twig->addExtension($sandbox); } + /** @var SandboxExtension $sandbox */ $sandbox = $this->twig->getExtension(SandboxExtension::class); $sandbox->enableSandbox(); } @@ -127,6 +128,7 @@ public function disableSandboxExtensionFromTwigEnvironment(): void { if ($this->twig->hasExtension(SandboxExtension::class)) { $sandbox = $this->twig->getExtension(SandboxExtension::class); + /** @var SandboxExtension $sandbox */ $sandbox->disableSandbox(); } } From c2715b64409389c362de6f060bcac3351f0c193a Mon Sep 17 00:00:00 2001 From: Divesh Pahuja Date: Tue, 11 Oct 2022 15:17:45 +0200 Subject: [PATCH 04/14] [Mail] Renderer email content twig templates in a sandbox --- .../DependencyInjection/Configuration.php | 37 +++++++++++++++++++ .../Resources/config/pimcore/default.yaml | 9 +++++ .../09_Upgrade_Notes/README.md | 15 ++++++++ lib/Mail.php | 14 +++---- .../TwigDefaultDelegatingEngine.php | 22 +++++------ .../ClassDefinition/Layout/Text.php | 18 +++++---- 6 files changed, 88 insertions(+), 27 deletions(-) diff --git a/bundles/CoreBundle/DependencyInjection/Configuration.php b/bundles/CoreBundle/DependencyInjection/Configuration.php index c0a8a8a603d..6aa66a3ba0e 100644 --- a/bundles/CoreBundle/DependencyInjection/Configuration.php +++ b/bundles/CoreBundle/DependencyInjection/Configuration.php @@ -180,6 +180,7 @@ public function getConfigTreeBuilder(): TreeBuilder $this->addCustomViewsNode($rootNode); $this->addGlossaryNode($rootNode); $this->buildRedirectsStatusCodes($rootNode); + $this->addTemplatingEngineNode($rootNode); return $treeBuilder; } @@ -2276,4 +2277,40 @@ private function addGlossaryNode(ArrayNodeDefinition $rootNode): void ->useAttributeAsKey('name') ->prototype('scalar'); } + + /** + * @param ArrayNodeDefinition $rootNode + */ + private function addTemplatingEngineNode(ArrayNodeDefinition $rootNode): void + { + $rootNode + ->children() + ->arrayNode('templating_engine') + ->addDefaultsIfNotSet() + ->children() + ->arrayNode('twig') + ->addDefaultsIfNotSet() + ->children() + ->arrayNode('security_policy') + ->children() + ->arrayNode('tags') + ->scalarPrototype()->end() + ->end() + ->arrayNode('filters') + ->scalarPrototype()->end() + ->end() + ->arrayNode('methods') + ->scalarPrototype()->end() + ->end() + ->arrayNode('properties') + ->scalarPrototype()->end() + ->end() + ->arrayNode('functions') + ->scalarPrototype()->end() + ->end() + ->end() + ->end() + ->end() + ->end(); + } } diff --git a/bundles/CoreBundle/Resources/config/pimcore/default.yaml b/bundles/CoreBundle/Resources/config/pimcore/default.yaml index 238b140ec2c..f071261a8c6 100644 --- a/bundles/CoreBundle/Resources/config/pimcore/default.yaml +++ b/bundles/CoreBundle/Resources/config/pimcore/default.yaml @@ -293,6 +293,15 @@ pimcore: 'abbr', 'option', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6' ] + pimcore: + templating_engine: + twig: + security_policy: + tags: ['set'] + filters: ['escape', 'trans'] + methods: [] + properties: [] + functions: ['path', 'asset'] presta_sitemap: # do not add properties by default defaults: diff --git a/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md b/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md index 845999d09a6..180e0305f10 100644 --- a/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md +++ b/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md @@ -1,5 +1,20 @@ # Upgrade Notes +## 10.5.8 +- [Twig] Sending mails and Dataobject Text Layouts, which allow rendering user controlled twig templates are now executed in a sandbox with restrictive security policies for tags, filters, functions and son on. + Please use following configuration to allow more tags, filters, properties, methods in template rendering: + ```yaml + pimcore: + templating_engine: + twig: + security_policy: + tags: ['set'] + filters: ['escape', 'trans'] + methods: [] + properties: [] + functions: ['path', 'asset'] + ``` + ## 10.5.0 - [Class Definitions] Resolving classes or services will no longer catch exceptions in Pimcore 11. Remove invalid references from class definitions. - [Sessions] Changed default value for `symfony.session.cookie_secure` to `auto` diff --git a/lib/Mail.php b/lib/Mail.php index bb8aeb6dd38..f2c08bdebe2 100644 --- a/lib/Mail.php +++ b/lib/Mail.php @@ -654,14 +654,14 @@ private function getDebugMailRecipients(array $recipients): array private function renderParams(string $string): string { $templatingEngine = \Pimcore::getContainer()->get('pimcore.templating.engine.delegating'); - $twig = $templatingEngine->getTwigEnvironment(true); - $template = $twig->createTemplate($string); + try { + $twig = $templatingEngine->getTwigEnvironment(true); + $template = $twig->createTemplate($string); - $rendered = $template->render($this->getParams()); - - $templatingEngine->disableSandboxExtensionFromTwigEnvironment(); - - return $rendered; + return $template->render($this->getParams()); + } finally { + $templatingEngine->disableSandboxExtensionFromTwigEnvironment(); + } } /** diff --git a/lib/Templating/TwigDefaultDelegatingEngine.php b/lib/Templating/TwigDefaultDelegatingEngine.php index 193a6c063d1..49c12e86d93 100644 --- a/lib/Templating/TwigDefaultDelegatingEngine.php +++ b/lib/Templating/TwigDefaultDelegatingEngine.php @@ -15,6 +15,7 @@ namespace Pimcore\Templating; +use Pimcore\Config; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Templating\DelegatingEngine as BaseDelegatingEngine; use Symfony\Component\Templating\EngineInterface; @@ -27,24 +28,16 @@ */ class TwigDefaultDelegatingEngine extends BaseDelegatingEngine { - /** - * @var Environment - */ - protected $twig; - /** * @var bool */ protected $delegate = false; /** - * @param Environment $twig * @param EngineInterface[] $engines */ - public function __construct(Environment $twig, array $engines = []) + public function __construct(protected Environment $twig, protected Config $config, array $engines = []) { - $this->twig = $twig; - parent::__construct($engines); } @@ -106,10 +99,13 @@ public function getTwigEnvironment($sandboxed = false): Environment { if ($sandboxed) { if (!$this->twig->hasExtension(SandboxExtension::class)) { - $tags = ['if', 'include', 'import', 'block', 'set', 'for']; - $filters = ['date', 'escape', 'trans', 'split', 'length', 'slice', 'lower', 'raw']; - $methods = $properties = []; - $functions = ['include', 'path', 'absolute_url', 'asset', 'is_granted']; + $securityPolicy = $this->config['templating_engine']['twig']['security_policy']; + + $tags = $securityPolicy['tags']; + $filters = $securityPolicy['filters']; + $methods = $securityPolicy['methods']; + $properties = $securityPolicy['properties']; + $functions = $securityPolicy['functions']; $policy = new SecurityPolicy($tags, $filters, $methods, $properties, $functions); $sandbox = new SandboxExtension($policy); diff --git a/models/DataObject/ClassDefinition/Layout/Text.php b/models/DataObject/ClassDefinition/Layout/Text.php index c0c51dc4277..20b346c947c 100644 --- a/models/DataObject/ClassDefinition/Layout/Text.php +++ b/models/DataObject/ClassDefinition/Layout/Text.php @@ -143,13 +143,17 @@ public function enrichLayoutDefinition(/* ?Concrete */ $object, /* array */ $con } $templatingEngine = \Pimcore::getContainer()->get('pimcore.templating.engine.delegating'); - $twig = $templatingEngine->getTwigEnvironment(); - $template = $twig->createTemplate($this->html); - $this->html = $template->render(array_merge($context, - [ - 'object' => $object, - ] - )); + try { + $twig = $templatingEngine->getTwigEnvironment(true); + $template = $twig->createTemplate($this->html); + $this->html = $template->render(array_merge($context, + [ + 'object' => $object, + ] + )); + } finally { + $templatingEngine->disableSandboxExtensionFromTwigEnvironment(); + } return $this; } From 7e67b0b5d1d60f2eb3908431bc91aa72068dbc03 Mon Sep 17 00:00:00 2001 From: Divesh Pahuja Date: Tue, 11 Oct 2022 16:46:41 +0200 Subject: [PATCH 05/14] [Mail] Renderer email content twig templates in a sandbox --- .../DependencyInjection/Configuration.php | 8 ++++++-- .../09_Upgrade_Notes/README.md | 14 ++++++++------ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/bundles/CoreBundle/DependencyInjection/Configuration.php b/bundles/CoreBundle/DependencyInjection/Configuration.php index 6aa66a3ba0e..583b319ccf2 100644 --- a/bundles/CoreBundle/DependencyInjection/Configuration.php +++ b/bundles/CoreBundle/DependencyInjection/Configuration.php @@ -2300,10 +2300,14 @@ private function addTemplatingEngineNode(ArrayNodeDefinition $rootNode): void ->scalarPrototype()->end() ->end() ->arrayNode('methods') - ->scalarPrototype()->end() + ->prototype('array') + ->prototype('scalar')->end() + ->end() ->end() ->arrayNode('properties') - ->scalarPrototype()->end() + ->prototype('array') + ->prototype('scalar')->end() + ->end() ->end() ->arrayNode('functions') ->scalarPrototype()->end() diff --git a/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md b/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md index 180e0305f10..1fa2095314f 100644 --- a/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md +++ b/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md @@ -7,12 +7,14 @@ pimcore: templating_engine: twig: - security_policy: - tags: ['set'] - filters: ['escape', 'trans'] - methods: [] - properties: [] - functions: ['path', 'asset'] + security_policy: + tags: ['if'] + filters: ['upper'] + methods: + Article: ['getTitle', 'getBody'] + properties: + Article: ['title', 'body'] + functions: ['include', 'path', 'range'] ``` ## 10.5.0 From a1d5e12fc8f741edd2f6fe0f1e1865c51b398826 Mon Sep 17 00:00:00 2001 From: Divesh Pahuja Date: Tue, 11 Oct 2022 16:48:31 +0200 Subject: [PATCH 06/14] [Mail] Renderer email content twig templates in a sandbox --- .../Resources/config/pimcore/default.yaml | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/bundles/CoreBundle/Resources/config/pimcore/default.yaml b/bundles/CoreBundle/Resources/config/pimcore/default.yaml index f071261a8c6..234e2e597cc 100644 --- a/bundles/CoreBundle/Resources/config/pimcore/default.yaml +++ b/bundles/CoreBundle/Resources/config/pimcore/default.yaml @@ -293,15 +293,14 @@ pimcore: 'abbr', 'option', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6' ] - pimcore: - templating_engine: - twig: - security_policy: - tags: ['set'] - filters: ['escape', 'trans'] - methods: [] - properties: [] - functions: ['path', 'asset'] + templating_engine: + twig: + security_policy: + tags: ['set'] + filters: ['escape', 'trans'] + methods: [] + properties: [] + functions: ['path', 'asset'] presta_sitemap: # do not add properties by default defaults: From fcad73de12034c00ae67fb399db559c5bd90bd0e Mon Sep 17 00:00:00 2001 From: Divesh Pahuja Date: Wed, 12 Oct 2022 08:37:22 +0200 Subject: [PATCH 07/14] Apply suggestions from code review Co-authored-by: Sebastian Blank --- bundles/CoreBundle/DependencyInjection/Configuration.php | 3 --- lib/Templating/TwigDefaultDelegatingEngine.php | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/bundles/CoreBundle/DependencyInjection/Configuration.php b/bundles/CoreBundle/DependencyInjection/Configuration.php index 583b319ccf2..13e1b420313 100644 --- a/bundles/CoreBundle/DependencyInjection/Configuration.php +++ b/bundles/CoreBundle/DependencyInjection/Configuration.php @@ -2278,9 +2278,6 @@ private function addGlossaryNode(ArrayNodeDefinition $rootNode): void ->prototype('scalar'); } - /** - * @param ArrayNodeDefinition $rootNode - */ private function addTemplatingEngineNode(ArrayNodeDefinition $rootNode): void { $rootNode diff --git a/lib/Templating/TwigDefaultDelegatingEngine.php b/lib/Templating/TwigDefaultDelegatingEngine.php index 49c12e86d93..5e53e49f399 100644 --- a/lib/Templating/TwigDefaultDelegatingEngine.php +++ b/lib/Templating/TwigDefaultDelegatingEngine.php @@ -95,7 +95,7 @@ public function isDelegate() return $this->delegate; } - public function getTwigEnvironment($sandboxed = false): Environment + public function getTwigEnvironment(bool $sandboxed = false): Environment { if ($sandboxed) { if (!$this->twig->hasExtension(SandboxExtension::class)) { From 8b8bb4391e573fe3db510eaaf2a578a19217fdbb Mon Sep 17 00:00:00 2001 From: Divesh Pahuja Date: Wed, 12 Oct 2022 09:32:10 +0200 Subject: [PATCH 08/14] Update lib/Templating/TwigDefaultDelegatingEngine.php Co-authored-by: Jacob Dreesen --- lib/Templating/TwigDefaultDelegatingEngine.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Templating/TwigDefaultDelegatingEngine.php b/lib/Templating/TwigDefaultDelegatingEngine.php index 5e53e49f399..fb76e5a2738 100644 --- a/lib/Templating/TwigDefaultDelegatingEngine.php +++ b/lib/Templating/TwigDefaultDelegatingEngine.php @@ -123,8 +123,8 @@ public function getTwigEnvironment(bool $sandboxed = false): Environment public function disableSandboxExtensionFromTwigEnvironment(): void { if ($this->twig->hasExtension(SandboxExtension::class)) { - $sandbox = $this->twig->getExtension(SandboxExtension::class); /** @var SandboxExtension $sandbox */ + $sandbox = $this->twig->getExtension(SandboxExtension::class); $sandbox->disableSandbox(); } } From eb0c1e29482e4b964cb70e850171c87982e47ea7 Mon Sep 17 00:00:00 2001 From: Divesh Pahuja Date: Fri, 14 Oct 2022 15:03:24 +0200 Subject: [PATCH 09/14] [Twig] Renderer user controlled twig templates in a sandbox - review changes #13347 --- .../DependencyInjection/Configuration.php | 39 ++++++++++--------- .../Resources/config/pimcore/default.yaml | 2 +- .../09_Upgrade_Notes/README.md | 2 +- lib/Mail.php | 14 +++++-- .../TwigDefaultDelegatingEngine.php | 2 +- .../ClassDefinition/Layout/Text.php | 8 ++++ 6 files changed, 42 insertions(+), 25 deletions(-) diff --git a/bundles/CoreBundle/DependencyInjection/Configuration.php b/bundles/CoreBundle/DependencyInjection/Configuration.php index 583b319ccf2..1e1b5a1f2dc 100644 --- a/bundles/CoreBundle/DependencyInjection/Configuration.php +++ b/bundles/CoreBundle/DependencyInjection/Configuration.php @@ -2291,26 +2291,29 @@ private function addTemplatingEngineNode(ArrayNodeDefinition $rootNode): void ->arrayNode('twig') ->addDefaultsIfNotSet() ->children() - ->arrayNode('security_policy') - ->children() - ->arrayNode('tags') - ->scalarPrototype()->end() - ->end() - ->arrayNode('filters') - ->scalarPrototype()->end() - ->end() - ->arrayNode('methods') - ->prototype('array') - ->prototype('scalar')->end() + ->arrayNode('sandbox_security_policy') + ->info('Whitelist tags, filters, methods, properties & functions for evaluating twig + templates in a sandbox environment e.g. used by Mailer & Text layout component.') + ->children() + ->arrayNode('tags') + ->scalarPrototype()->end() ->end() - ->end() - ->arrayNode('properties') - ->prototype('array') - ->prototype('scalar')->end() + ->arrayNode('filters') + ->scalarPrototype()->end() + ->end() + ->arrayNode('methods') + ->prototype('array') + ->prototype('scalar')->end() + ->end() + ->end() + ->arrayNode('properties') + ->prototype('array') + ->prototype('scalar')->end() + ->end() + ->end() + ->arrayNode('functions') + ->scalarPrototype()->end() ->end() - ->end() - ->arrayNode('functions') - ->scalarPrototype()->end() ->end() ->end() ->end() diff --git a/bundles/CoreBundle/Resources/config/pimcore/default.yaml b/bundles/CoreBundle/Resources/config/pimcore/default.yaml index 234e2e597cc..e2f3ae1afd7 100644 --- a/bundles/CoreBundle/Resources/config/pimcore/default.yaml +++ b/bundles/CoreBundle/Resources/config/pimcore/default.yaml @@ -295,7 +295,7 @@ pimcore: templating_engine: twig: - security_policy: + sandbox_security_policy: tags: ['set'] filters: ['escape', 'trans'] methods: [] diff --git a/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md b/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md index 1fa2095314f..305e9940e95 100644 --- a/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md +++ b/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md @@ -7,7 +7,7 @@ pimcore: templating_engine: twig: - security_policy: + sandbox_security_policy: tags: ['if'] filters: ['upper'] methods: diff --git a/lib/Mail.php b/lib/Mail.php index f2c08bdebe2..6228ca0ed9f 100644 --- a/lib/Mail.php +++ b/lib/Mail.php @@ -28,6 +28,7 @@ use Symfony\Component\Mime\Header\Headers; use Symfony\Component\Mime\Header\MailboxListHeader; use Symfony\Component\Mime\Part\AbstractPart; +use Twig\Sandbox\SecurityError; class Mail extends Email { @@ -651,7 +652,7 @@ private function getDebugMailRecipients(array $recipients): array * * @return string */ - private function renderParams(string $string): string + private function renderParams(string $string, string $context): string { $templatingEngine = \Pimcore::getContainer()->get('pimcore.templating.engine.delegating'); try { @@ -659,6 +660,11 @@ private function renderParams(string $string): string $template = $twig->createTemplate($string); return $template->render($this->getParams()); + } catch (SecurityError $e) { + Logger::err((string) $e); + + throw new \Exception(sprintf("Failed rendering the %s: %s. Please check your twig sandbox security policy or contact the administrator.", + $context, substr($e->getMessage(),0, strpos($e->getMessage(), ' in "__string')))); } finally { $templatingEngine->disableSandboxExtensionFromTwigEnvironment(); } @@ -680,7 +686,7 @@ public function getSubjectRendered() } if ($subject) { - return $this->renderParams($subject); + return $this->renderParams($subject, 'subject'); } return ''; @@ -711,7 +717,7 @@ public function getBodyHtmlRendered() $content = null; if ($html) { - $content = $this->renderParams($html); + $content = $this->renderParams($html, 'body'); // modifying the content e.g set absolute urls... $content = MailHelper::embedAndModifyCss($content, $this->getDocument()); @@ -735,7 +741,7 @@ public function getBodyTextRendered() //if the content was manually set with $obj->text(); this content will be used if ($text) { - $content = $this->renderParams($text); + $content = $this->renderParams($text, 'body'); } else { //creating text version from html email try { diff --git a/lib/Templating/TwigDefaultDelegatingEngine.php b/lib/Templating/TwigDefaultDelegatingEngine.php index 49c12e86d93..482c56be6ae 100644 --- a/lib/Templating/TwigDefaultDelegatingEngine.php +++ b/lib/Templating/TwigDefaultDelegatingEngine.php @@ -99,7 +99,7 @@ public function getTwigEnvironment($sandboxed = false): Environment { if ($sandboxed) { if (!$this->twig->hasExtension(SandboxExtension::class)) { - $securityPolicy = $this->config['templating_engine']['twig']['security_policy']; + $securityPolicy = $this->config['templating_engine']['twig']['sandbox_security_policy']; $tags = $securityPolicy['tags']; $filters = $securityPolicy['filters']; diff --git a/models/DataObject/ClassDefinition/Layout/Text.php b/models/DataObject/ClassDefinition/Layout/Text.php index 20b346c947c..5887afb336d 100644 --- a/models/DataObject/ClassDefinition/Layout/Text.php +++ b/models/DataObject/ClassDefinition/Layout/Text.php @@ -15,8 +15,10 @@ namespace Pimcore\Model\DataObject\ClassDefinition\Layout; +use Pimcore\Logger; use Pimcore\Model; use Pimcore\Model\DataObject\Concrete; +use Twig\Sandbox\SecurityError; class Text extends Model\DataObject\ClassDefinition\Layout implements Model\DataObject\ClassDefinition\Data\LayoutDefinitionEnrichmentInterface { @@ -151,6 +153,12 @@ public function enrichLayoutDefinition(/* ?Concrete */ $object, /* array */ $con 'object' => $object, ] )); + } catch (SecurityError $e) { + Logger::err((string) $e); + + $this->html = sprintf("

Error

Failed rendering the template: %s. + Please check your twig sandbox security policy or contact the administrator.", + substr($e->getMessage(),0, strpos($e->getMessage(), ' in "__string'))); } finally { $templatingEngine->disableSandboxExtensionFromTwigEnvironment(); } From aa5d1534784b99860da300390f672ff14d91299c Mon Sep 17 00:00:00 2001 From: Divesh Pahuja Date: Tue, 25 Oct 2022 16:58:36 +0200 Subject: [PATCH 10/14] [Twig] Renderer user controlled twig templates in a sandbox - use custom security policy to whitelist object properties and methods execution by default #13347 --- .../DependencyInjection/Configuration.php | 12 +-- .../Resources/config/pimcore/default.yaml | 2 - .../01_Dynamic_Text_Labels.md | 15 +++ .../25_Email_Framework/README.md | 15 +++ .../09_Upgrade_Notes/README.md | 4 - .../TwigDefaultDelegatingEngine.php | 6 +- lib/Twig/Sandbox/SecurityPolicy.php | 91 +++++++++++++++++++ 7 files changed, 124 insertions(+), 21 deletions(-) create mode 100644 lib/Twig/Sandbox/SecurityPolicy.php diff --git a/bundles/CoreBundle/DependencyInjection/Configuration.php b/bundles/CoreBundle/DependencyInjection/Configuration.php index 6ab5a69e178..4ca2f9a4f42 100644 --- a/bundles/CoreBundle/DependencyInjection/Configuration.php +++ b/bundles/CoreBundle/DependencyInjection/Configuration.php @@ -2289,7 +2289,7 @@ private function addTemplatingEngineNode(ArrayNodeDefinition $rootNode): void ->addDefaultsIfNotSet() ->children() ->arrayNode('sandbox_security_policy') - ->info('Whitelist tags, filters, methods, properties & functions for evaluating twig + ->info('Whitelist tags, filters & functions for evaluating twig templates in a sandbox environment e.g. used by Mailer & Text layout component.') ->children() ->arrayNode('tags') @@ -2298,16 +2298,6 @@ private function addTemplatingEngineNode(ArrayNodeDefinition $rootNode): void ->arrayNode('filters') ->scalarPrototype()->end() ->end() - ->arrayNode('methods') - ->prototype('array') - ->prototype('scalar')->end() - ->end() - ->end() - ->arrayNode('properties') - ->prototype('array') - ->prototype('scalar')->end() - ->end() - ->end() ->arrayNode('functions') ->scalarPrototype()->end() ->end() diff --git a/bundles/CoreBundle/Resources/config/pimcore/default.yaml b/bundles/CoreBundle/Resources/config/pimcore/default.yaml index e2f3ae1afd7..063a3578e1d 100644 --- a/bundles/CoreBundle/Resources/config/pimcore/default.yaml +++ b/bundles/CoreBundle/Resources/config/pimcore/default.yaml @@ -298,8 +298,6 @@ pimcore: sandbox_security_policy: tags: ['set'] filters: ['escape', 'trans'] - methods: [] - properties: [] functions: ['path', 'asset'] presta_sitemap: # do not add properties by default diff --git a/doc/Development_Documentation/05_Objects/01_Object_Classes/03_Layout_Elements/01_Dynamic_Text_Labels.md b/doc/Development_Documentation/05_Objects/01_Object_Classes/03_Layout_Elements/01_Dynamic_Text_Labels.md index d5126a7e979..133cc3aa6f3 100644 --- a/doc/Development_Documentation/05_Objects/01_Object_Classes/03_Layout_Elements/01_Dynamic_Text_Labels.md +++ b/doc/Development_Documentation/05_Objects/01_Object_Classes/03_Layout_Elements/01_Dynamic_Text_Labels.md @@ -67,3 +67,18 @@ Here is an example of Twig content in htmleditor source edit mode: ![Template Class Definition](../../../img/dynamic_textlabel_3.png) ![Template editmode](../../../img/dynamic_textlabel_4.png) + +### Sandbox Restrictions +Dynamic Text renders user controlled twig templates in a sandbox with restrictive +security policies for tags, filters, functions and son on. Please use following configuration to allow more tags, +filters, properties, methods in template rendering: + +```yaml + pimcore: + templating_engine: + twig: + sandbox_security_policy: + tags: ['if'] + filters: ['upper'] + functions: ['include', 'path'] +``` \ No newline at end of file diff --git a/doc/Development_Documentation/19_Development_Tools_and_Details/25_Email_Framework/README.md b/doc/Development_Documentation/19_Development_Tools_and_Details/25_Email_Framework/README.md index 692a3d0cb6c..94b221cff1b 100644 --- a/doc/Development_Documentation/19_Development_Tools_and_Details/25_Email_Framework/README.md +++ b/doc/Development_Documentation/19_Development_Tools_and_Details/25_Email_Framework/README.md @@ -98,3 +98,18 @@ $mail->bcc("bcc@pimcore.org"); $mail->html("some rich text"); $mail->send(); ``` + +## Sandbox Restrictions +Sending mails renders user controlled twig templates in a sandbox with restrictive +security policies for tags, filters, functions and son on. Please use following configuration to allow more tags, +filters, properties, methods in template rendering: + +```yaml + pimcore: + templating_engine: + twig: + sandbox_security_policy: + tags: ['if'] + filters: ['upper'] + functions: ['include', 'path'] +``` \ No newline at end of file diff --git a/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md b/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md index ef22414cb8e..33426de1173 100644 --- a/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md +++ b/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md @@ -10,10 +10,6 @@ sandbox_security_policy: tags: ['if'] filters: ['upper'] - methods: - Article: ['getTitle', 'getBody'] - properties: - Article: ['title', 'body'] functions: ['include', 'path', 'range'] ``` diff --git a/lib/Templating/TwigDefaultDelegatingEngine.php b/lib/Templating/TwigDefaultDelegatingEngine.php index a22f0f2121d..b2bfa0bd061 100644 --- a/lib/Templating/TwigDefaultDelegatingEngine.php +++ b/lib/Templating/TwigDefaultDelegatingEngine.php @@ -21,7 +21,7 @@ use Symfony\Component\Templating\EngineInterface; use Twig\Environment; use Twig\Extension\SandboxExtension; -use Twig\Sandbox\SecurityPolicy; +use Pimcore\Twig\Sandbox\SecurityPolicy; /** * @internal @@ -103,11 +103,9 @@ public function getTwigEnvironment(bool $sandboxed = false): Environment $tags = $securityPolicy['tags']; $filters = $securityPolicy['filters']; - $methods = $securityPolicy['methods']; - $properties = $securityPolicy['properties']; $functions = $securityPolicy['functions']; - $policy = new SecurityPolicy($tags, $filters, $methods, $properties, $functions); + $policy = new SecurityPolicy($tags, $filters, $functions); $sandbox = new SandboxExtension($policy); $this->twig->addExtension($sandbox); } diff --git a/lib/Twig/Sandbox/SecurityPolicy.php b/lib/Twig/Sandbox/SecurityPolicy.php new file mode 100644 index 00000000000..832e5230214 --- /dev/null +++ b/lib/Twig/Sandbox/SecurityPolicy.php @@ -0,0 +1,91 @@ + + */ +final class SecurityPolicy implements SecurityPolicyInterface +{ + private array $allowedTags; + private array $allowedFilters; + private array $allowedFunctions; + + public function __construct(array $allowedTags = [], array $allowedFilters = [], array $allowedFunctions = []) + { + $this->allowedTags = $allowedTags; + $this->allowedFilters = $allowedFilters; + $this->allowedFunctions = $allowedFunctions; + } + + public function setAllowedTags(array $tags) + { + $this->allowedTags = $tags; + } + + public function setAllowedFilters(array $filters) + { + $this->allowedFilters = $filters; + } + + public function setAllowedFunctions(array $functions) + { + $this->allowedFunctions = $functions; + } + + public function checkSecurity($tags, $filters, $functions): void + { + foreach ($tags as $tag) { + if (!\in_array($tag, $this->allowedTags)) { + throw new SecurityNotAllowedTagError(sprintf('Tag "%s" is not allowed.', $tag), $tag); + } + } + + foreach ($filters as $filter) { + if (!\in_array($filter, $this->allowedFilters)) { + throw new SecurityNotAllowedFilterError(sprintf('Filter "%s" is not allowed.', $filter), $filter); + } + } + + foreach ($functions as $function) { + //check if a function is allowed or a pimcore twig functions + if (!\in_array($function, $this->allowedFunctions) && !str_starts_with($function, 'pimcore_')) { + throw new SecurityNotAllowedFunctionError(sprintf('Function "%s" is not allowed.', $function), $function); + } + } + } + + public function checkMethodAllowed($obj, $method): void + { + //do not perform any checks + } + + public function checkPropertyAllowed($obj, $method): void + { + //do not perform any checks + } +} From 4fe1b1bbb3dfb18b0fee63be620e6aee55dd00d3 Mon Sep 17 00:00:00 2001 From: Divesh Pahuja Date: Thu, 27 Oct 2022 12:45:16 +0200 Subject: [PATCH 11/14] [Twig] Renderer user controlled twig templates in a sandbox - review changes #13347 --- .../PimcoreCoreExtension.php | 5 ++++ .../Resources/config/templating_twig.yaml | 10 ++++++++ .../TwigDefaultDelegatingEngine.php | 25 ++++--------------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/bundles/CoreBundle/DependencyInjection/PimcoreCoreExtension.php b/bundles/CoreBundle/DependencyInjection/PimcoreCoreExtension.php index 3432636547a..7fffc7dcfb1 100644 --- a/bundles/CoreBundle/DependencyInjection/PimcoreCoreExtension.php +++ b/bundles/CoreBundle/DependencyInjection/PimcoreCoreExtension.php @@ -95,6 +95,11 @@ public function loadInternal(array $config, ContainerBuilder $container) $container->setParameter('pimcore.documents.web_to_print.default_controller_print_page', $config['documents']['web_to_print']['default_controller_print_page']); $container->setParameter('pimcore.documents.web_to_print.default_controller_print_container', $config['documents']['web_to_print']['default_controller_print_container']); + //twig security policy whitelist config + $container->setParameter('pimcore.templating.twig.sandbox_security_policy.tags', $config['templating_engine']['twig']['sandbox_security_policy']['tags']); + $container->setParameter('pimcore.templating.twig.sandbox_security_policy.filters', $config['templating_engine']['twig']['sandbox_security_policy']['filters']); + $container->setParameter('pimcore.templating.twig.sandbox_security_policy.functions', $config['templating_engine']['twig']['sandbox_security_policy']['functions']); + // register pimcore config on container // TODO is this bad practice? // TODO only extract what we need as parameter? diff --git a/bundles/CoreBundle/Resources/config/templating_twig.yaml b/bundles/CoreBundle/Resources/config/templating_twig.yaml index 5655bfd3040..c08d1ad024e 100644 --- a/bundles/CoreBundle/Resources/config/templating_twig.yaml +++ b/bundles/CoreBundle/Resources/config/templating_twig.yaml @@ -84,4 +84,14 @@ services: Twig\Extra\String\StringExtension: ~ + Pimcore\Twig\Sandbox\SecurityPolicy: + arguments: + $allowedTags: '%pimcore.templating.twig.sandbox_security_policy.tags%' + $allowedFilters: '%pimcore.templating.twig.sandbox_security_policy.filters%' + $allowedFunctions: '%pimcore.templating.twig.sandbox_security_policy.functions%' + + Twig\Extension\SandboxExtension: + arguments: + $policy: Pimcore\Twig\Sandbox\SecurityPolicy + Pimcore\Twig\Extension\AdminExtension: ~ diff --git a/lib/Templating/TwigDefaultDelegatingEngine.php b/lib/Templating/TwigDefaultDelegatingEngine.php index b2bfa0bd061..1177a3f8dc8 100644 --- a/lib/Templating/TwigDefaultDelegatingEngine.php +++ b/lib/Templating/TwigDefaultDelegatingEngine.php @@ -21,7 +21,6 @@ use Symfony\Component\Templating\EngineInterface; use Twig\Environment; use Twig\Extension\SandboxExtension; -use Pimcore\Twig\Sandbox\SecurityPolicy; /** * @internal @@ -98,21 +97,9 @@ public function isDelegate() public function getTwigEnvironment(bool $sandboxed = false): Environment { if ($sandboxed) { - if (!$this->twig->hasExtension(SandboxExtension::class)) { - $securityPolicy = $this->config['templating_engine']['twig']['sandbox_security_policy']; - - $tags = $securityPolicy['tags']; - $filters = $securityPolicy['filters']; - $functions = $securityPolicy['functions']; - - $policy = new SecurityPolicy($tags, $filters, $functions); - $sandbox = new SandboxExtension($policy); - $this->twig->addExtension($sandbox); - } - /** @var SandboxExtension $sandbox */ - $sandbox = $this->twig->getExtension(SandboxExtension::class); - $sandbox->enableSandbox(); + $sandboxExtension = $this->twig->getExtension(SandboxExtension::class); + $sandboxExtension->enableSandbox(); } return $this->twig; @@ -120,11 +107,9 @@ public function getTwigEnvironment(bool $sandboxed = false): Environment public function disableSandboxExtensionFromTwigEnvironment(): void { - if ($this->twig->hasExtension(SandboxExtension::class)) { - /** @var SandboxExtension $sandbox */ - $sandbox = $this->twig->getExtension(SandboxExtension::class); - $sandbox->disableSandbox(); - } + /** @var SandboxExtension $sandbox */ + $sandboxExtension = $this->twig->getExtension(SandboxExtension::class); + $sandboxExtension->disableSandbox(); } /** From 506f53abf5f13a17fa73e08fde707c4a555b0ecf Mon Sep 17 00:00:00 2001 From: Divesh Pahuja Date: Thu, 27 Oct 2022 12:51:50 +0200 Subject: [PATCH 12/14] [Twig] Renderer user controlled twig templates in a sandbox - fix phpstan #13347 --- lib/Templating/TwigDefaultDelegatingEngine.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Templating/TwigDefaultDelegatingEngine.php b/lib/Templating/TwigDefaultDelegatingEngine.php index 1177a3f8dc8..4abd7b38082 100644 --- a/lib/Templating/TwigDefaultDelegatingEngine.php +++ b/lib/Templating/TwigDefaultDelegatingEngine.php @@ -97,7 +97,7 @@ public function isDelegate() public function getTwigEnvironment(bool $sandboxed = false): Environment { if ($sandboxed) { - /** @var SandboxExtension $sandbox */ + /** @var SandboxExtension $sandboxExtension */ $sandboxExtension = $this->twig->getExtension(SandboxExtension::class); $sandboxExtension->enableSandbox(); } @@ -107,7 +107,7 @@ public function getTwigEnvironment(bool $sandboxed = false): Environment public function disableSandboxExtensionFromTwigEnvironment(): void { - /** @var SandboxExtension $sandbox */ + /** @var SandboxExtension $sandboxExtension */ $sandboxExtension = $this->twig->getExtension(SandboxExtension::class); $sandboxExtension->disableSandbox(); } From 125a3e7ad3073ca6b5e91b18bcf0ed8f16453426 Mon Sep 17 00:00:00 2001 From: Divesh Pahuja Date: Thu, 27 Oct 2022 13:21:54 +0200 Subject: [PATCH 13/14] [Twig] Renderer user controlled twig templates in a sandbox - fix service definition #13347 --- bundles/CoreBundle/Resources/config/templating_twig.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bundles/CoreBundle/Resources/config/templating_twig.yaml b/bundles/CoreBundle/Resources/config/templating_twig.yaml index c08d1ad024e..59e02698e25 100644 --- a/bundles/CoreBundle/Resources/config/templating_twig.yaml +++ b/bundles/CoreBundle/Resources/config/templating_twig.yaml @@ -91,7 +91,8 @@ services: $allowedFunctions: '%pimcore.templating.twig.sandbox_security_policy.functions%' Twig\Extension\SandboxExtension: + class: Twig\Extension\SandboxExtension arguments: - $policy: Pimcore\Twig\Sandbox\SecurityPolicy + $policy: '@Pimcore\Twig\Sandbox\SecurityPolicy' Pimcore\Twig\Extension\AdminExtension: ~ From a67d9647fb7a1dd6b7b7feec867855577bec34ba Mon Sep 17 00:00:00 2001 From: Divesh Pahuja Date: Thu, 27 Oct 2022 14:53:17 +0200 Subject: [PATCH 14/14] [Twig] Renderer user controlled twig templates in a sandbox - docs typo #13347 --- .../03_Layout_Elements/01_Dynamic_Text_Labels.md | 3 +-- .../25_Email_Framework/README.md | 3 +-- .../23_Installation_and_Upgrade/09_Upgrade_Notes/README.md | 4 ++-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/doc/Development_Documentation/05_Objects/01_Object_Classes/03_Layout_Elements/01_Dynamic_Text_Labels.md b/doc/Development_Documentation/05_Objects/01_Object_Classes/03_Layout_Elements/01_Dynamic_Text_Labels.md index 133cc3aa6f3..91d535b9376 100644 --- a/doc/Development_Documentation/05_Objects/01_Object_Classes/03_Layout_Elements/01_Dynamic_Text_Labels.md +++ b/doc/Development_Documentation/05_Objects/01_Object_Classes/03_Layout_Elements/01_Dynamic_Text_Labels.md @@ -70,8 +70,7 @@ Here is an example of Twig content in htmleditor source edit mode: ### Sandbox Restrictions Dynamic Text renders user controlled twig templates in a sandbox with restrictive -security policies for tags, filters, functions and son on. Please use following configuration to allow more tags, -filters, properties, methods in template rendering: +security policies for tags, filters & functions. Please use following configuration to allow more in template rendering: ```yaml pimcore: diff --git a/doc/Development_Documentation/19_Development_Tools_and_Details/25_Email_Framework/README.md b/doc/Development_Documentation/19_Development_Tools_and_Details/25_Email_Framework/README.md index 94b221cff1b..c18ae513ed4 100644 --- a/doc/Development_Documentation/19_Development_Tools_and_Details/25_Email_Framework/README.md +++ b/doc/Development_Documentation/19_Development_Tools_and_Details/25_Email_Framework/README.md @@ -101,8 +101,7 @@ $mail->send(); ## Sandbox Restrictions Sending mails renders user controlled twig templates in a sandbox with restrictive -security policies for tags, filters, functions and son on. Please use following configuration to allow more tags, -filters, properties, methods in template rendering: +security policies for tags, filters & functions. Please use following configuration to allow more in template rendering: ```yaml pimcore: diff --git a/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md b/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md index 33426de1173..2eb6d2c7967 100644 --- a/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md +++ b/doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md @@ -1,8 +1,8 @@ # Upgrade Notes ## 10.5.8 -- [Twig] Sending mails and Dataobject Text Layouts, which allow rendering user controlled twig templates are now executed in a sandbox with restrictive security policies for tags, filters, functions and son on. - Please use following configuration to allow more tags, filters, properties, methods in template rendering: +- [Twig] Sending mails and Dataobject Text Layouts, which allow rendering user controlled twig templates are now executed in a sandbox with restrictive security policies for tags, filters, functions. + Please use following configuration to allow more in template rendering: ```yaml pimcore: templating_engine: