WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 9 months ago

#37941 assigned enhancement

add rel="noopener noreferrer" to any target="_blank"

Reported by: Presskopp Owned by: nicolapeluchetti
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Security Keywords: good-first-bug has-patch
Focuses: Cc:
PR Number:

Description

This is a following ticket to #36809

It's about making these links more secure where/when they are used.

see:
https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vulnerability-ever/

Attachments (2)

37941.diff (71.9 KB) - added by Presskopp 3 years ago.
37941.2.diff (4.2 KB) - added by nicola.peluchetti 2 years ago.

Download all attachments as: .zip

Change History (26)

@Presskopp
3 years ago

#1 @Presskopp
3 years ago

  • Keywords has-patch added

Patch is simply adding it to any found instance.

I'm sure it needs more (or less) to be done,

but I don't know which php-files or js-files to touch, to generate this tags for each link with target="_blank" set.

Last edited 3 years ago by Presskopp (previous) (diff)

#2 @Presskopp
3 years ago

  • Keywords needs-patch added; has-patch removed

#3 @Ipstenu
3 years ago

  • Resolution set to duplicate
  • Status changed from new to closed

This doesn't need to be a separate ticket at this time.

#36809 is just going to transmute from fix A to fix B :) Happens all the time.

#4 @swissspidy
3 years ago

  • Milestone Awaiting Review deleted
  • Version trunk deleted

#5 @kevinlangleyjr
3 years ago

  • Resolution duplicate deleted
  • Status changed from closed to reopened

Per comment https://core.trac.wordpress.org/ticket/36809#comment:15 and https://core.trac.wordpress.org/ticket/36809#comment:10, this should be a separate ticket and patch than the original ticket.

Reopening since I've added a patch for the other ticket, #36809, and this is still valid per the above mentioned comments.

#6 @SergeyBiryukov
3 years ago

  • Milestone set to Awaiting Review

#7 @swissspidy
2 years ago

#41061 was marked as a duplicate.

#8 @galbaras
2 years ago

Will this fix be covering the Links functions for backward compatibility? I have those on several sites.

Google Lighthouse is flagging this as "not best practice", which means that WordPress sites are likely having ranking points taken off for insecure external links.

#9 @iandunn
2 years ago

  • Component changed from General to Security
  • Keywords good-first-bug added
  • Type changed from defect (bug) to enhancement

This ticket was mentioned in Slack in #core by canepa. View the logs.


2 years ago

#11 @Presskopp
2 years ago

I want to report that if you add a link to the toolbar like so

$args = array(
...
'href'   => 'https://example.com',
'meta'   => array(
...
'target' => '_blank',
...
// Add link
$wp_admin_bar->add_node($args);

it will not have these attributes..

#12 @nicola.peluchetti
2 years ago

@iandunn I added a patch to fix

$wp_admin_bar->add_node

behaviour so that, when adding a link with 'target' => '_blank', it will add 'noopener noreferrer' to the rel ( creating it if it doesn't exists ).

#13 @galbaras
2 years ago

Awesome! Will this fix be covering the old Links functions for backward compatibility?

#14 @nicola.peluchetti
2 years ago

@galbaras what do you mean as "Old links function"?I've posted a fix for $wp_admin_bar->add_node($args); while the original patch addresses issues in html links. Can you try to use both and see if it fixes your issues?If it doesn't, just add the issue here with an explanation

#15 follow-up: @swissspidy
2 years ago

I guess they mean the old Link Manager. That one is deprecated though, so it won‘t get any updates.

#16 @nicola.peluchetti
2 years ago

  • Keywords has-patch added; needs-patch removed

#17 in reply to: ↑ 15 @galbaras
23 months ago

Replying to nicola.peluchetti and swissspidy:

@galbaras what do you mean as "Old links function"?I've posted a fix for $wp_admin_bar->add_node($args); while the original patch addresses issues in html links. Can you try to use both and see if it fixes your issues?If it doesn't, just add the issue here with an explanation

I guess they mean the old Link Manager. That one is deprecated though, so it won‘t get any updates.

Yes, that's what I meant. Just wasn't sure if the same functions were used.

As far as I can see, the link manager and the link widget are still there, running on one of my sites.

Either way, another place to mention is menus, where links can be set to open in a new window. I hope this change will cover them.

#18 @swissspidy
23 months ago

#43187 was marked as a duplicate.

#19 @DrewAPicture
22 months ago

  • Owner set to nicolapeluchetti
  • Status changed from reopened to assigned

Assigning to mark the re-opened good-first-bug as "claimed".

#23 follow-up: @afercia
9 months ago

As mentioned on #46421, I'd suggest to investigate on the actual usefulness of noreferrer:

#24 in reply to: ↑ 23 @audrasjb
9 months ago

Replying to afercia:

As mentioned on #46421, I'd suggest to investigate on the actual usefulness of noreferrer:

As mentionned on #43280 I think both of the rel attributes should be added, in order to keep some consistency with https://developer.wordpress.org/reference/functions/wp_targeted_link_rel/

If we want to remove noreferrer, I believe we should remove it from that function first ;)

Last edited 9 months ago by audrasjb (previous) (diff)
Note: See TracTickets for help on using tickets.