Skip to content

DOMPurify: Allow `target` attribute for anchor tags in a safe context

Dheeraj Joshi requested to merge djadmin-dompurify-target-attr into master

Related issue - gitlab-ui#1427 (closed), #365372 (closed)

What does this MR do and why?

Context

DOMPurify forbids target attribute by default - https://github.com/cure53/DOMPurify/issues/317.

This resulted in an issue where the external links are no longer opens in a new tab, when the components were switched to using DOMPurify or v-safe-html.

Proposal

With this MR,

  1. we've added hooks to preserve the target attribute values, and
  2. also sets secure context when target=_blank because links to cross-origin destinations are unsafe.

Few decision notes:

  1. Why not allowed the target by default? This would open up vulnerabilities in all the places where rel attributes are not present
  2. Why do we use beforeSanitizeAttributes? This is to preserve the original target attribute value

Screenshots or screen recordings

<!-- original -->
<a href="https://example.com" target="_blank">link</a>

<!-- sanitised -->
<a href="https://example.com" target="_blank" rel="noopener noreferrer">link</a>

How to set up and validate locally

  1. Add an external link to any issue / MR description
  2. The link should now be opened in another tab

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Dheeraj Joshi

Merge request reports