Skip to content
Permalink
Browse files Browse the repository at this point in the history
[Mail] Renderer email content twig templates in a sandbox (#13347)
* [Mail] Renderer email content twig templates in a sandbox

* [Mail] Renderer email content twig templates in a sandbox

* [Mail] Renderer email content twig templates in a sandbox

* [Mail] Renderer email content twig templates in a sandbox

* [Mail] Renderer email content twig templates in a sandbox

* [Mail] Renderer email content twig templates in a sandbox

* Apply suggestions from code review

Co-authored-by: Sebastian Blank <blank@data-factory.net>

* Update lib/Templating/TwigDefaultDelegatingEngine.php

Co-authored-by: Jacob Dreesen <j.dreesen@neusta.de>

* [Twig] Renderer user controlled twig templates in a sandbox - review changes #13347

* [Twig] Renderer user controlled twig templates in a sandbox - use custom security policy to whitelist object properties and methods execution by default #13347

* [Twig] Renderer user controlled twig templates in a sandbox - review changes #13347

* [Twig] Renderer user controlled twig templates in a sandbox - fix phpstan #13347

* [Twig] Renderer user controlled twig templates in a sandbox - fix service definition #13347

* [Twig] Renderer user controlled twig templates in a sandbox - docs typo #13347

Co-authored-by: Sebastian Blank <blank@data-factory.net>
Co-authored-by: Jacob Dreesen <j.dreesen@neusta.de>
  • Loading branch information
3 people committed Oct 27, 2022
1 parent c07e8ab commit 43aa34e
Show file tree
Hide file tree
Showing 11 changed files with 238 additions and 28 deletions.
31 changes: 31 additions & 0 deletions bundles/CoreBundle/DependencyInjection/Configuration.php
Expand Up @@ -180,6 +180,7 @@ public function getConfigTreeBuilder(): TreeBuilder
$this->addCustomViewsNode($rootNode);
$this->addGlossaryNode($rootNode);
$this->buildRedirectsStatusCodes($rootNode);
$this->addTemplatingEngineNode($rootNode);

return $treeBuilder;
}
Expand Down Expand Up @@ -2276,4 +2277,34 @@ private function addGlossaryNode(ArrayNodeDefinition $rootNode): void
->useAttributeAsKey('name')
->prototype('scalar');
}

private function addTemplatingEngineNode(ArrayNodeDefinition $rootNode): void
{
$rootNode
->children()
->arrayNode('templating_engine')
->addDefaultsIfNotSet()
->children()
->arrayNode('twig')
->addDefaultsIfNotSet()
->children()
->arrayNode('sandbox_security_policy')
->info('Whitelist tags, filters & functions for evaluating twig
templates in a sandbox environment e.g. used by Mailer & Text layout component.')
->children()
->arrayNode('tags')
->scalarPrototype()->end()
->end()
->arrayNode('filters')
->scalarPrototype()->end()
->end()
->arrayNode('functions')
->scalarPrototype()->end()
->end()
->end()
->end()
->end()
->end()
->end();
}
}
Expand Up @@ -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?
Expand Down
6 changes: 6 additions & 0 deletions bundles/CoreBundle/Resources/config/pimcore/default.yaml
Expand Up @@ -293,6 +293,12 @@ pimcore:
'abbr', 'option', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6'
]

templating_engine:
twig:
sandbox_security_policy:
tags: ['set']
filters: ['escape', 'trans']
functions: ['path', 'asset']
presta_sitemap:
# do not add properties by default
defaults:
Expand Down
11 changes: 11 additions & 0 deletions bundles/CoreBundle/Resources/config/templating_twig.yaml
Expand Up @@ -84,4 +84,15 @@ 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:
class: Twig\Extension\SandboxExtension
arguments:
$policy: '@Pimcore\Twig\Sandbox\SecurityPolicy'

Pimcore\Twig\Extension\AdminExtension: ~
Expand Up @@ -67,3 +67,17 @@ 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. Please use following configuration to allow more in template rendering:

```yaml
pimcore:
templating_engine:
twig:
sandbox_security_policy:
tags: ['if']
filters: ['upper']
functions: ['include', 'path']
```
Expand Up @@ -98,3 +98,17 @@ $mail->bcc("bcc@pimcore.org");
$mail->html("<b>some</b> rich text");
$mail->send();
```

## Sandbox Restrictions
Sending mails renders user controlled twig templates 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:
twig:
sandbox_security_policy:
tags: ['if']
filters: ['upper']
functions: ['include', 'path']
```
@@ -1,6 +1,18 @@
# 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.
Please use following configuration to allow more in template rendering:
```yaml
pimcore:
templating_engine:
twig:
sandbox_security_policy:
tags: ['if']
filters: ['upper']
functions: ['include', 'path', 'range']
```

- [Nginx] Static pages nginx config has been updated to fix the issue for home static page generation. please adapt the following configuration:
```nginx
map $args $static_page_root {
Expand Down
26 changes: 18 additions & 8 deletions lib/Mail.php
Expand Up @@ -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
{
Expand Down Expand Up @@ -651,13 +652,22 @@ 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');
$twig = $templatingEngine->getTwigEnvironment();
$template = $twig->createTemplate($string);

return $template->render($this->getParams());
try {
$twig = $templatingEngine->getTwigEnvironment(true);
$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();
}
}

/**
Expand All @@ -676,7 +686,7 @@ public function getSubjectRendered()
}

if ($subject) {
return $this->renderParams($subject);
return $this->renderParams($subject, 'subject');
}

return '';
Expand Down Expand Up @@ -707,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());
Expand All @@ -731,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 {
Expand Down
30 changes: 17 additions & 13 deletions lib/Templating/TwigDefaultDelegatingEngine.php
Expand Up @@ -15,34 +15,28 @@

namespace Pimcore\Templating;

use Pimcore\Config;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Templating\DelegatingEngine as BaseDelegatingEngine;
use Symfony\Component\Templating\EngineInterface;
use Twig\Environment;
use Twig\Extension\SandboxExtension;

/**
* @internal
*/
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);
}

Expand Down Expand Up @@ -100,14 +94,24 @@ public function isDelegate()
return $this->delegate;
}

/**
* @return Environment
*/
public function getTwigEnvironment(): Environment
public function getTwigEnvironment(bool $sandboxed = false): Environment
{
if ($sandboxed) {
/** @var SandboxExtension $sandboxExtension */
$sandboxExtension = $this->twig->getExtension(SandboxExtension::class);
$sandboxExtension->enableSandbox();
}

return $this->twig;
}

public function disableSandboxExtensionFromTwigEnvironment(): void
{
/** @var SandboxExtension $sandboxExtension */
$sandboxExtension = $this->twig->getExtension(SandboxExtension::class);
$sandboxExtension->disableSandbox();
}

/**
* @param string $view
* @param array $parameters
Expand Down
91 changes: 91 additions & 0 deletions lib/Twig/Sandbox/SecurityPolicy.php
@@ -0,0 +1,91 @@
<?php

declare(strict_types=1);

/**
* Pimcore
*
* This source file is available under two different licenses:
* - GNU General Public License version 3 (GPLv3)
* - Pimcore Commercial License (PCL)
* Full copyright and license information is available in
* LICENSE.md which is distributed with this source code.
*
* @copyright Copyright (c) Pimcore GmbH (http://www.pimcore.org)
* @license http://www.pimcore.org/license GPLv3 and PCL
*/

namespace Pimcore\Twig\Sandbox;

use Twig\Sandbox\SecurityNotAllowedFilterError;
use Twig\Sandbox\SecurityNotAllowedFunctionError;
use Twig\Sandbox\SecurityNotAllowedTagError;
use Twig\Sandbox\SecurityPolicyInterface;

/**
* Note: Reused to disable checks on object methods and properties.
*
* Represents a security policy which need to be enforced when sandbox mode is enabled.
*
* @author Fabien Potencier <fabien@symfony.com>
*/
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
}
}

0 comments on commit 43aa34e

Please sign in to comment.