Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SECURITY: Token should not be stored in the localstorage to limit XSS vulnerability #24

Open
gagarine opened this issue Jul 7, 2021 · 9 comments

Comments

@gagarine
Copy link

gagarine commented Jul 7, 2021

The token should be added in a httponly cookie and only the refresh token should be stored.

See https://dev.to/cotter/localstorage-vs-cookies-all-you-need-to-know-about-storing-jwt-tokens-securely-in-the-front-end-15id

@lil5
Copy link

lil5 commented Aug 14, 2021

I advise that the current version is deprecated in npm to show this issue. Until a fix is published it will continue to warn developers during install.

https://docs.npmjs.com/deprecating-and-undeprecating-packages-or-package-versions

@gagarine gagarine changed the title Token should not be stored in the localstorage to limit XSS vulnerability SECURITY: Token should not be stored in the localstorage to limit XSS vulnerability Aug 15, 2021
@wecoon
Copy link

wecoon commented Aug 26, 2021

Is this library still supported? This needs fixing

@mvanroon
Copy link
Contributor

mvanroon commented Aug 26, 2021

Is this library still supported? This needs fixing

Feel free to create pull request mate

@mvanroon
Copy link
Contributor

mvanroon commented Aug 26, 2021

Relevant:

https://pragmaticwebsecurity.com/articles/oauthoidc/localstorage-xss.html

Step 1: don’t worry too much about storage. I often get questions from developers asking how to store tokens securely. If you are unsure how to handle token storage, use LocalStorage. It will save you a massive amount of time, which you will need for the next steps. If you know what you are doing, use in-memory storage or the Web Worker option. It makes an attack harder, but it never solves the root problem.

@revmischa
Copy link
Member

Totally agree. Patches welcome! Should bump major version or add backwards-compatible support for reading localStorage though.

@germansokolov13
Copy link

This idea is debatable and will require backend code change. Cookies are prone to a CSRF attack and XSS even without localStorage, if done, will deal a lot of damage.

@revmischa
Copy link
Member

This idea is debatable and will require backend code change. Cookies are prone to a CSRF attack and XSS even without localStorage, if done, will deal a lot of damage.

XSS would apply to localStorage as well, right? The difference is cookies can be set HTTPOnly?

@germansokolov13
Copy link

@revmischa XSS, if it already happened, applies to localStorage and cookies that don't have HTTPOnly flag. But on the other hand CSRF applies to any cookie unless it has flag sameSite set to either Lax or Strict but not to None, as long as given browser supports sameSite flag(93% of users if we trust numbers from caniuse.com).

With cookies without sameSite we need to go extra mile to create protection against CSRF with one technique or another. localStorage is always protected against CSRF by design. sameSite will not work if we use CORS though. And if XSS did happen then even with well protected cookies we still have our site compromised because XSS is very dangerous anyways. It's not like you don't need to filter user input just because you have cookies protected.

@greenyas
Copy link
Contributor

Do not look for the problem in the localStorage, instead safeguard you app against XSS attacks. Not an issue of this lib, neither localStorage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants