From edc1bccaac0af84f68b82ee70560a3aef8efe36c Mon Sep 17 00:00:00 2001 From: Florent Morselli Date: Sun, 25 May 2025 15:21:16 +0200 Subject: [PATCH] [HtmlSanitizer] Add support for securing target="_blank" links Introduce `ensureSafeBlankTarget` to automatically add `rel="noopener noreferrer"` to `` elements with `target="_blank"`, mitigating reverse tabnabbing risks. The `allowUnsafeBlankTargets` method allows opting out of this behavior if needed. Included tests validate the new functionality. --- .../HtmlSanitizer/HtmlSanitizerConfig.php | 22 ++++++++ .../Tests/HtmlSanitizerAllTest.php | 53 +++++++++++++++++++ .../HtmlSanitizer/Visitor/DomVisitor.php | 10 ++++ .../HtmlSanitizer/Visitor/Node/Node.php | 5 +- 4 files changed, 86 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Component/HtmlSanitizer/HtmlSanitizerConfig.php b/src/Symfony/Component/HtmlSanitizer/HtmlSanitizerConfig.php index f7b0b0523bc43..13bb13bdd886a 100644 --- a/src/Symfony/Component/HtmlSanitizer/HtmlSanitizerConfig.php +++ b/src/Symfony/Component/HtmlSanitizer/HtmlSanitizerConfig.php @@ -87,6 +87,12 @@ class HtmlSanitizerConfig */ private bool $allowRelativeMedias = false; + /** + * When set to true, the sanitizer will ensure that any element with target="_blank" is also accompanied + * by rel="noopener noreferrer" to prevent potential reverse tabnabbing vulnerabilities. + */ + private bool $ensureSafeBlankTarget = true; + /** * Should the URL in the sanitized document be transformed to HTTPS if they are using HTTP. */ @@ -257,6 +263,17 @@ public function allowRelativeMedias(bool $allowRelativeMedias = true): static return $clone; } + /** + * Allows the use of the target="_blank" attribute without rel="noopener noreferrer". + */ + public function allowUnsafeBlankTargets(): static + { + $clone = clone $this; + $clone->ensureSafeBlankTarget = false; + + return $clone; + } + /** * Transforms URLs using the HTTP scheme to use the HTTPS scheme instead. */ @@ -529,6 +546,11 @@ public function getAllowRelativeMedias(): bool return $this->allowRelativeMedias; } + public function getEnsureSafeBlankTarget(): bool + { + return $this->ensureSafeBlankTarget; + } + public function getForceHttpsUrls(): bool { return $this->forceHttpsUrls; diff --git a/src/Symfony/Component/HtmlSanitizer/Tests/HtmlSanitizerAllTest.php b/src/Symfony/Component/HtmlSanitizer/Tests/HtmlSanitizerAllTest.php index 8699879f67bfd..c5a9b64b0d601 100644 --- a/src/Symfony/Component/HtmlSanitizer/Tests/HtmlSanitizerAllTest.php +++ b/src/Symfony/Component/HtmlSanitizer/Tests/HtmlSanitizerAllTest.php @@ -600,4 +600,57 @@ public function testAllowByDefault() $sanitizer = new HtmlSanitizer($config); self::assertSame('

Hello

', $sanitizer->sanitize('

Hello World

')); } + + /** + * @dataProvider provideTargetBlank + */ + public function testSafeTargetBlank(array $allowedAttributes, string $input, string $expectedSanitized) + { + $sanitizer = new HtmlSanitizer((new HtmlSanitizerConfig()) + ->allowElement('a', $allowedAttributes) + ->allowLinkHosts(['trusted.com']) + ); + + $sanitized = $sanitizer->sanitize($input); + + $this->assertSame($expectedSanitized, $sanitized); + } + + public static function provideTargetBlank() + { + return [ + // No rel attribute + [ + ['href', 'target', 'rel'], + 'Lorem ipsum', + 'Lorem ipsum', + ], + + // Normal tags + [ + ['href', 'target', 'rel'], + 'Lorem ipsum', + 'Lorem ipsum', + ], + + // Normal tags + [ + ['href', 'target'], + 'Lorem ipsum', + 'Lorem ipsum', + ], + // Missing noreferrer + [ + ['href', 'target', 'rel'], + 'Lorem ipsum', + 'Lorem ipsum', + ], + // Missing noopener + [ + ['href', 'target', 'rel'], + 'Lorem ipsum', + 'Lorem ipsum', + ], + ]; + } } diff --git a/src/Symfony/Component/HtmlSanitizer/Visitor/DomVisitor.php b/src/Symfony/Component/HtmlSanitizer/Visitor/DomVisitor.php index e6d34a0967b79..953fd4684c53d 100644 --- a/src/Symfony/Component/HtmlSanitizer/Visitor/DomVisitor.php +++ b/src/Symfony/Component/HtmlSanitizer/Visitor/DomVisitor.php @@ -184,5 +184,15 @@ private function setAttributes(string $domNodeName, \DOMNode $domNode, Node $nod $node->setAttribute($name, $value); } } + if ('a' === $domNodeName + && $this->config->getEnsureSafeBlankTarget() + && '_blank' === strtolower($node->getAttribute('target') ?? '') + ) { $rel = $node->getAttribute('rel') ?? ''; + $parts = explode(' ', strtolower($rel)); + if (!in_array('noopener', $parts, true) || !in_array('noreferrer', $parts, true)) { + $parts = array_unique(array_merge($parts, ['noopener', 'noreferrer'])); + $node->setAttribute('rel', trim(implode(' ', $parts))); + } + } } } diff --git a/src/Symfony/Component/HtmlSanitizer/Visitor/Node/Node.php b/src/Symfony/Component/HtmlSanitizer/Visitor/Node/Node.php index d25803776f80d..17f5068146e81 100644 --- a/src/Symfony/Component/HtmlSanitizer/Visitor/Node/Node.php +++ b/src/Symfony/Component/HtmlSanitizer/Visitor/Node/Node.php @@ -58,10 +58,7 @@ public function getAttribute(string $name): ?string public function setAttribute(string $name, ?string $value): void { - // Always use only the first declaration (ease sanitization) - if (!\array_key_exists($name, $this->attributes)) { - $this->attributes[$name] = $value; - } + $this->attributes[$name] = $value; } public function addChild(NodeInterface $node): void