Make WordPress Core

Opened 8 years ago

Closed 21 months ago

Last modified 3 months ago

#37941 closed enhancement (wontfix)

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

Reported by: presskopp's profile Presskopp Owned by: nicolapeluchetti's profile nicolapeluchetti
Milestone: Priority: normal
Severity: normal Version:
Component: Security Keywords: good-first-bug has-patch close
Focuses: Cc:

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 8 years ago.
37941.2.diff (4.2 KB) - added by nicola.peluchetti 6 years ago.

Download all attachments as: .zip

Change History (32)

@Presskopp
8 years ago

#1 @Presskopp
8 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 8 years ago by Presskopp (previous) (diff)

#2 @Presskopp
8 years ago

  • Keywords needs-patch added; has-patch removed

#3 @Ipstenu
8 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
8 years ago

  • Milestone Awaiting Review deleted
  • Version trunk deleted

#5 @kevinlangleyjr
7 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
7 years ago

  • Milestone set to Awaiting Review

#7 @swissspidy
7 years ago

#41061 was marked as a duplicate.

#8 @galbaras
6 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
6 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.


6 years ago

#11 @Presskopp
6 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
6 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
6 years ago

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

#14 @nicola.peluchetti
6 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
6 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
6 years ago

  • Keywords has-patch added; needs-patch removed

#17 in reply to: ↑ 15 @galbaras
6 years 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
6 years ago

#43187 was marked as a duplicate.

#19 @DrewAPicture
6 years ago

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

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

#23 follow-ups: @afercia
5 years ago

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

#24 in reply to: ↑ 23 @audrasjb
5 years 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 5 years ago by audrasjb (previous) (diff)

#25 @SergeyBiryukov
4 years ago

#49313 was marked as a duplicate.

#26 in reply to: ↑ 23 @SergeyBiryukov
3 years ago

Replying to afercia:

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

Follow-up: #49558

#27 @azaozz
3 years ago

  • Keywords close added

Adding noopener to links with target="_blank" is becoming unnecessary. Most browsers now imply noopener in that case. See #53843.

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


21 months ago

#29 @peterwilsoncc
21 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

The Can I Use data for a: target="_blank" implies rel="noopener" behavior show 89.5% of traffic uses a browser implying the behavior at the time of writing.

The browsers that do not imply it are generally unsupported by WordPress. I tend to agree with @azaozz that this can be closed.

I'll close this as wontfix as the ticket was valid at the time of the report. Browsers have simply fixed it in the meantime for the entire web.

#30 @sabernhardt
3 months ago

#59856 was marked as a duplicate.

Note: See TracTickets for help on using tickets.