Skip to content

Prevent window opener vulnerability with space shortcut#2506

Merged
Alkarex merged 2 commits intoFreshRSS:devfrom
ArthurHoaro:hotfix/new-window-vulnerability
Aug 21, 2019
Merged

Prevent window opener vulnerability with space shortcut#2506
Alkarex merged 2 commits intoFreshRSS:devfrom
ArthurHoaro:hotfix/new-window-vulnerability

Conversation

@ArthurHoaro
Copy link
Contributor

@ArthurHoaro ArthurHoaro commented Aug 21, 2019

This change fixes a vulnerability introduced by window.open() on untrusted sources. It reproduces the effect of rel="noreferrer" with JS.

Cross browser solution from: https://stackoverflow.com/a/40593743

Related to #1245

Reproduction

tested with Firefox 68

  1. Add this RSS feed https://www.ecyseo.net/feed/rss
  2. Open the 2nd link "À propos de la faille de sécurité liée à target="_blank" using the space key shortcut.
  3. Click on the first of three links "http://bookmarks.ecyseo.net"

Current behaviour: the FreshRSS tab changes.
Expected behaviour: no effect on FreshRSS

This change fixes a vulnerability introduced by `window.open()` on untrusted sources. It reproduces the effect of `rel="noreferrer"` with JS.

Cross browser solution from: https://stackoverflow.com/a/40593743

## Reproduction

> tested with Firefox 68

  1. Add this RSS feed
  2. Open the 2nd link "À propos de la faille de sécurité liée à target="_blank" **using the space key shortcut**.
  3. Click on the first of three links "http://bookmarks.ecyseo.net"

Current behaviour: the FreshRSS tab changes.
Expected behaviour: no effect on FreshRSS
@Alkarex Alkarex added this to the 1.15.0 milestone Aug 21, 2019
@Alkarex
Copy link
Member

Alkarex commented Aug 21, 2019

This looks fine. I have just made a little change in the case of popup blockers, in which case window.open() returns null.
Tested in IE11, Edge, Firefox

@Alkarex
Copy link
Member

Alkarex commented Aug 21, 2019

I have just found a bug in IE11 + Edge with the Space shortcut. Fix coming
Edit: fix #2507

Alkarex added a commit to Alkarex/FreshRSS that referenced this pull request Aug 21, 2019
@Alkarex Alkarex merged commit 3f8804f into FreshRSS:dev Aug 21, 2019
@Alkarex
Copy link
Member

Alkarex commented Aug 21, 2019

@ArthurHoaro Thanks again, and please add yourself to https://github.com/FreshRSS/FreshRSS/blob/dev/CREDITS.md

@Alkarex
Copy link
Member

Alkarex commented Oct 3, 2024

#6862

Alkarex added a commit to Alkarex/FreshRSS that referenced this pull request Dec 8, 2024
@Alkarex Alkarex mentioned this pull request Dec 8, 2024
@Alkarex
Copy link
Member

Alkarex commented Dec 8, 2024

#7077

Alkarex added a commit that referenced this pull request Dec 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants