From cee4df87883bf89441a776cce104b986f39adacf Mon Sep 17 00:00:00 2001 From: Bob den Otter Date: Thu, 23 Jul 2020 14:41:24 +0200 Subject: [PATCH] Make Sanitiser obey allowed tags and attributes from `config.yaml` --- composer.json | 4 ++-- src/Entity/ContentExtrasTrait.php | 5 +++++ src/Entity/Field.php | 4 +--- src/Twig/ContentExtension.php | 14 +++++++++++++- src/Utils/Sanitiser.php | 24 +++++++++++++++++++++--- symfony.lock | 3 +++ 6 files changed, 45 insertions(+), 9 deletions(-) diff --git a/composer.json b/composer.json index e627f3010..45989a530 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,6 @@ "drupol/composer-packages": "^1.1", "embed/embed": "^3.4", "erusev/parsedown": "^1.7", - "ezyang/htmlpurifier": "^4.12", "fzaninotto/faker": "^1.9", "knplabs/doctrine-behaviors": "^2.0.3", "knplabs/knp-menu-bundle": "^3.0", @@ -70,7 +69,8 @@ "ua-parser/uap-php": "^3.9", "webimpress/safe-writer": "^2.0", "webmozart/path-util": "^2.3", - "webonyx/graphql-php": "^0.13" + "webonyx/graphql-php": "^0.13", + "xemlock/htmlpurifier-html5": "^0.1.11" }, "conflict": { "symfony/symfony": "*" diff --git a/src/Entity/ContentExtrasTrait.php b/src/Entity/ContentExtrasTrait.php index 0f607e91a..3035dcf5e 100644 --- a/src/Entity/ContentExtrasTrait.php +++ b/src/Entity/ContentExtrasTrait.php @@ -45,4 +45,9 @@ public function getExtras(): array 'feature' => $this->contentExtension->getSpecialFeature($content), ]); } + + public function sanitise(string $string): string + { + return $this->contentExtension->sanitise($string); + } } diff --git a/src/Entity/Field.php b/src/Entity/Field.php index 8deff99aa..79caa5438 100644 --- a/src/Entity/Field.php +++ b/src/Entity/Field.php @@ -8,7 +8,6 @@ use ApiPlatform\Core\Annotation\ApiResource; use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\SearchFilter; use Bolt\Configuration\Content\FieldType; -use Bolt\Utils\Sanitiser; use Doctrine\ORM\Mapping as ORM; use Knp\DoctrineBehaviors\Contract\Entity\TranslatableInterface; use Knp\DoctrineBehaviors\Model\Translatable\TranslatableTrait; @@ -229,8 +228,7 @@ public function getTwigValue() $value = $this->getParsedValue(); if (is_string($value) && $this->getDefinition()->get('sanitise')) { - $sanitiser = new Sanitiser(); - $value = $sanitiser->clean($value); + $value = $this->getContent()->sanitise($value); } if (is_string($value) && $this->getDefinition()->get('allow_twig')) { diff --git a/src/Twig/ContentExtension.php b/src/Twig/ContentExtension.php index 0d55b5642..b2dcb4ab8 100644 --- a/src/Twig/ContentExtension.php +++ b/src/Twig/ContentExtension.php @@ -22,6 +22,7 @@ use Bolt\Utils\ContentHelper; use Bolt\Utils\Excerpt; use Bolt\Utils\Html; +use Bolt\Utils\Sanitiser; use Pagerfanta\Pagerfanta; use Symfony\Component\Finder\Finder; use Symfony\Component\HttpFoundation\Request; @@ -79,6 +80,9 @@ class ContentExtension extends AbstractExtension /** @var Notifications */ private $notifications; + /** @var Sanitiser */ + private $sanitiser; + public function __construct( UrlGeneratorInterface $urlGenerator, ContentRepository $contentRepository, @@ -91,7 +95,8 @@ public function __construct( TranslatorInterface $translator, Canonical $canonical, ContentHelper $contentHelper, - Notifications $notifications + Notifications $notifications, + Sanitiser $sanitiser ) { $this->urlGenerator = $urlGenerator; $this->contentRepository = $contentRepository; @@ -105,6 +110,7 @@ public function __construct( $this->canonical = $canonical; $this->contentHelper = $contentHelper; $this->notifications = $notifications; + $this->sanitiser = $sanitiser; } /** @@ -132,6 +138,7 @@ public function getFilters(): array new TwigFilter('allow_twig', [$this, 'allowTwig'], $env), new TwigFilter('status_options', [$this, 'statusOptions']), new TwigFilter('feature', [$this, 'getSpecialFeature']), + new TwigFilter('sanitise', [$this, 'sanitise']), ]; } @@ -722,4 +729,9 @@ public function isHomepageListing(ContentType $contentType): bool return false; } + + public function sanitise(string $html) + { + return $this->sanitiser->clean($html); + } } diff --git a/src/Utils/Sanitiser.php b/src/Utils/Sanitiser.php index 2eb0f42b2..1a41cba88 100644 --- a/src/Utils/Sanitiser.php +++ b/src/Utils/Sanitiser.php @@ -4,16 +4,34 @@ namespace Bolt\Utils; +use Bolt\Configuration\Config; + class Sanitiser { private $purifier; - public function __construct() + public function __construct(?Config $config = null) { - $purifierConfig = \HTMLPurifier_Config::create([ - // Disable caching + $purifierConfig = \HTMLPurifier_HTML5Config::create([ 'Cache.DefinitionImpl' => null, + 'HTML.SafeIframe' => true, ]); + + if ($config) { + $allowedTags = implode(',', $config->get('general/htmlcleaner/allowed_tags')->all()); + $allowedAttributes = implode(',', $config->get('general/htmlcleaner/allowed_attributes')->all()); + $purifierConfig->set('HTML.AllowedElements', $allowedTags); + $purifierConfig->set('HTML.AllowedAttributes', $allowedAttributes); + } + + $definition = $purifierConfig->maybeGetRawHTMLDefinition(); + $definition->addElement('super', 'Inline', 'Flow', 'Common', []); + $definition->addElement('sub', 'Inline', 'Flow', 'Common', []); + $definition->addAttribute('a', 'value', 'Text'); + $definition->addAttribute('a', 'frameborder', 'Text'); + $definition->addAttribute('a', 'allowfullscreen', 'Text'); + $definition->addAttribute('a', 'scrolling', 'Text'); + $this->purifier = new \HTMLPurifier($purifierConfig); } diff --git a/symfony.lock b/symfony.lock index 2b14366b7..310a8e811 100644 --- a/symfony.lock +++ b/symfony.lock @@ -1110,6 +1110,9 @@ "willdurand/negotiation": { "version": "v2.3.1" }, + "xemlock/htmlpurifier-html5": { + "version": "v0.1.11" + }, "zendframework/zend-code": { "version": "3.4.1" },