Skip to content

[HtmlSanitizer] Add support for securing target="_blank" links #60539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: 7.4
Choose a base branch
from
+86 −4
Open

[HtmlSanitizer] Add support for securing target="_blank" links #60539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/Symfony/Component/HtmlSanitizer/HtmlSanitizerConfig.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ class HtmlSanitizerConfig
*/
private bool $allowRelativeMedias = false;

/**
* When set to true, the sanitizer will ensure that any <a> 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.
*/
Expand Down Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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;
Expand Down
53 changes: 53 additions & 0 deletions src/Symfony/Component/HtmlSanitizer/Tests/HtmlSanitizerAllTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,4 +600,57 @@ public function testAllowByDefault()
$sanitizer = new HtmlSanitizer($config);
self::assertSame('<foo><div><p><a>Hello</a></p></div></foo>', $sanitizer->sanitize('<foo data-attr="value"><div class="foo"><p><a target="_blank">Hello<span> World</span></a></p></div></foo>'));
}

/**
* @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'],
'<a href="https://trusted.com" target="_blank">Lorem ipsum</a>',
'<a href="https://trusted.com" target="_blank" rel="noopener noreferrer">Lorem ipsum</a>',
],

// Normal tags
[
['href', 'target', 'rel'],
'<a href="https://trusted.com" target="_blank" rel="external">Lorem ipsum</a>',
'<a href="https://trusted.com" target="_blank" rel="external noopener noreferrer">Lorem ipsum</a>',
],

// Normal tags
[
['href', 'target'],
'<a href="https://trusted.com" target="_blank" rel="external">Lorem ipsum</a>',
'<a href="https://trusted.com" target="_blank" rel="noopener noreferrer">Lorem ipsum</a>',
],
// Missing noreferrer
[
['href', 'target', 'rel'],
'<a href="https://trusted.com" target="_blank" rel="noopener">Lorem ipsum</a>',
'<a href="https://trusted.com" target="_blank" rel="noopener noreferrer">Lorem ipsum</a>',
],
// Missing noopener
[
['href', 'target', 'rel'],
'<a href="https://trusted.com" target="_blank" rel="noreferrer">Lorem ipsum</a>',
'<a href="https://trusted.com" target="_blank" rel="noreferrer noopener">Lorem ipsum</a>',
],
];
}
}
10 changes: 10 additions & 0 deletions src/Symfony/Component/HtmlSanitizer/Visitor/DomVisitor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
}
}
}
}
5 changes: 1 addition & 4 deletions src/Symfony/Component/HtmlSanitizer/Visitor/Node/Node.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like we really understand what this means before removing.

if (!\array_key_exists($name, $this->attributes)) {
$this->attributes[$name] = $value;
}
$this->attributes[$name] = $value;
}

public function addChild(NodeInterface $node): void
Expand Down