Merge pull request #1648 from bolt/fix/sanitiser

Make Sanitiser obey allowed tags and attributes from `config.yaml`
bolt · Jul 23, 2020 · c4ac86d · c4ac86d
2 parents ed2fb43 + cee4df8
commit c4ac86d
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -1110,6 +1110,9 @@
     "willdurand/negotiation": {
         "version": "v2.3.1"
     },
+    "xemlock/htmlpurifier-html5": {
+        "version": "v0.1.11"
+    },
     "zendframework/zend-code": {
         "version": "3.4.1"
     },