monlovesmango / astral

Nostr client made with Quasar
https://astral.ninja
MIT License
101 stars 38 forks source link

add `rel='noopener noreferrer'` in links #80

Closed ghost closed 1 year ago

ghost commented 1 year ago

Added rel='noopener noreferrer' in HTML links as best practice to avoid any vulnerabilities.

More info: https://cheatsheetseries.owasp.org/cheatsheets/HTML5_Security_Cheat_Sheet.html#tabnabbing

monlovesmango commented 1 year ago

hey I don't think you need this for links that have target='_blank' as it opens a net new page...?

ghost commented 1 year ago

hey I don't think you need this for links that have target='_blank' as it opens a net new page...?

This works only when you have target=’_blank’ and new page that opens in new tab could be used to change content on main page.

This is a proof of concept although vulnerable page has target='_blank' rel='opener' in this case:

vulnerable.html:

<!DOCTYPE html>
<html>
<body>
<h1>Victim Site</h1>
<a href="http://127.0.0.1:8000/malicious.html" target="_blank" rel="opener">Controlled by the attacker</a>
</body>
</html>

malicious.html:

<!DOCTYPE html>
<html>
 <body>
  <script>
  window.opener.location = "http://127.0.0.1:8000/malicious_redir.html";
  </script>
 </body>
</html>

malicious_redir.html:

<!DOCTYPE html>
<html>
<body>
<h1>New Malicious Site</h1>
</body>
</html>

Create the following pages in a folder and run a web server with python3 -m http.server. Then, access http://127.0.0.1:8000/vulnerable.html, click on the link and note how the original website URL changes.

If you remove the rel="opener" attribute, the default behavior will depend on the browser. In some cases, the new browsing context will still be considered related to the original one, while in others it may not. So the best practice to avoid vulnerability is to add rel="noopener noreferrer".

rel="noopener" prevents the new page from being able to access the window.opener property and ensures it runs in a separate process. rel="noreferrer" has the same effect but also prevents the Referer header from being sent to the new page.

ghost commented 1 year ago

https://user-images.githubusercontent.com/94559964/210490940-04b1b030-c4cf-4457-af56-b5fb9d53b692.mp4

SuperPhatArrow commented 1 year ago

ACK - Code changes reviewed. Please accept PR.

Basically these keywords prevent the browser from passing info from the page a user is navigating from to the new tab they opened about where it was opened from. Not just that, but if the site is malicious, it has access to Astral's window object, meaning potentially stolen keys from window.localStorage.keys and access to window.nostr if they are using a browser extension to manage keys. If I am wrong about access to localStorage, there is definitely access to window.location which opens the user to a phishing attack of a site that looks like Astral saying "oops, something went wrong, please paste in your private key again".

While this was fixed in a spec change in 2018 so that modern browsers only pass this info on when a link specifies the opener keyword, you should not assume that all users have updated browsers and you should ALWAYS include them with target="_blank" for user privacy and security.

See MDN noreferrer & MDN noopener. Here is another explainer.

monlovesmango commented 1 year ago

ok thank you for explaining this to me more in depth. and thanks for the pr @1440000bytes