Skip to content

fix(security): add rel="noopener noreferrer" to links opening a new tab - #723

Open
Bhuvanesh66 wants to merge 1 commit into
containers:mainfrom
Bhuvanesh66:fix/reverse-tabnabbing-noopener
Open

fix(security): add rel="noopener noreferrer" to links opening a new tab#723
Bhuvanesh66 wants to merge 1 commit into
containers:mainfrom
Bhuvanesh66:fix/reverse-tabnabbing-noopener

Conversation

@Bhuvanesh66

Copy link
Copy Markdown

Concisely describe the change

External links that open in a new tab were missing a rel attribute. When a link uses target="_blank" with no rel, the page that opens gets a working window.opener handle back to podman.io and can navigate the original tab somewhere else. That is reverse tabnabbing, and it also leaks the referrer.

On main there are 12 target="_blank" links under src/, and only one of them (BlogArticlesList) sets rel.

The root of it is the shared Link utility in src/components/utilities/Link/index.tsx. It accepts a target prop but never rendered rel, so any caller passing target="_blank" was affected. The 404 page does exactly that. I changed Link to default rel to noopener noreferrer when target is _blank, and a caller can still pass its own rel if it needs something different. Fixing it there covers the current caller and any future one, rather than leaving the next person to remember.

The rest are plain <a> tags and are fixed directly:

  • ThankYouSection - the seven sponsor logo links
  • ArticleCard - both article links. These open URLs that come back from the blog.podman.io API, so I would least want those left open.
  • Testimonial - the featured link
  • blog/2018-10-10-checkpoint-restore.md - one asciinema link

BlogArticlesList already set rel correctly and is not touched. No styling, markup structure, or behaviour changes beyond the added attribute.

Fixes: #671
Fixes: #576

Before screenshot / screen recording

Not a visual change, so there is nothing useful to screenshot. On main:

$ grep -rEc "target=[\"']_blank[\"']" src/ | awk -F: '{s+=$2} END{print s}'
12
$ grep -rc 'rel="noopener noreferrer"' src/ | awk -F: '{s+=$2} END{print s}'
1

After screenshot / screen recording

With this branch, the same count is 11 literal occurrences plus the 404 page link, which now gets rel from the Link component at render time.

I checked the generated HTML rather than just the source. After yarn build, across every page in build/:

$ grep -rohE '<a[^>]*target="_blank"[^>]*>' build --include=*.html | wc -l
10630
$ grep -rohE '<a[^>]*target="_blank"[^>]*>' build --include=*.html | grep -vc 'noopener'
0

For example the 404 page link, which is the one that goes through Link:

<a href="" target="_blank" rel="noopener noreferrer" class="font-bold text-blue-700 ...">

and a sponsor logo:

<a href="https://www.suse.com" target="_blank" rel="noopener noreferrer" class="mx-4 mb-4 inline-block ...">

yarn build completes successfully and build/index.html is produced, which is what the PR workflow checks.

Checklist

Ensure you have completed the following checklist for your pull request to be reviewed:

  • Certify you wrote the patch or otherwise have the right to pass it on as an open-source patch by signing all
    commits. (git commit -s). (If needed, use git commit -s --amend). The author email must match
    the sign-off email address. See CONTRIBUTING.md
    for more information.
  • Referenced issues using Fixes: #00000 in commit message (if applicable)
  • PR description, commit message, and GitHub comments are human-written, per LLM Policy

@TomSweeneyRedHat @ashley-cui when you get a chance, could one of you take a look? Happy to split the Link change out into its own commit if you would rather review it separately.

Links rendered with target="_blank" did not set a rel attribute, so the
page that opens keeps a working window.opener handle back to podman.io
and can navigate the original tab elsewhere. That is reverse tabnabbing,
and it also leaks the referrer.

The shared Link utility was the root of it. It accepts a target prop but
never rendered rel, so every caller passing target="_blank" was affected,
including the 404 page. It now defaults rel to "noopener noreferrer" when
target is "_blank", while still letting a caller pass an explicit rel.

The remaining raw anchors are fixed directly: the seven sponsor logos in
ThankYouSection, both article links in ArticleCard (these open URLs
returned by the blog.podman.io API), the featured link in Testimonial,
and one asciinema link in an older blog post.

Verified against the generated HTML rather than only the source: after
yarn build, no anchor with target="_blank" is left without rel on any
page in build/.

Fixes: containers#671
Fixes: containers#576
Signed-off-by: Bhuvanesh66 <bhuvaneshms60@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant