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

window.history.replaceState vulnerabilities potential in foundation.reveal #11380

Closed
abdulhamid-alattar opened this issue Jul 4, 2018 · 6 comments
Labels

Comments

@abdulhamid-alattar
Copy link

What should happen?

What happens instead?

manipulation of browser history

Possible Solution

window.history.replaceState('', document.title, window.location.href.replace(#${this.id}, ''));
replace state shouldnt be assigned directly from the href url

Test Case and/or Steps to Reproduce

Test Case:
Discoverd by Burp Suite , you can manipulate the history by passing query string or hash

How to reproduce:
1.
2.
3.

Context

Your Environment

  • Foundation version(s) used: 6.4.3
  • Browser(s) name and version(s): chrome
  • Operating System and version (desktop or mobile): mac

Checklist (all required):

  • [yes ] I have read and follow the CONTRIBUTING.md document.
  • [ yes] There are no other issues similar to this one.
  • [yes ] The issue title is descriptive.
  • [ yes] The template is correctly filled.
@DanielRuf
Copy link
Contributor

window.history.replaceState('', document.title, window.location.href.replace(#${this.id}, ''));
replace state shouldnt be assigned directly from the href url

This is not a security vulnerability imho. Also this depends on the html and is the same way how it is done in other frameworks and libraries who do deeplinking.

@DanielRuf
Copy link
Contributor

Discoverd by Burp Suite , you can manipulate the history by passing query string or hash

Not true as this is only controllable from the html, not in the browser url. Also this just replaces the URL.

@DanielRuf
Copy link
Contributor

{this.id}

This is the id of the reveal.

@ncoden ncoden added the security label Jul 4, 2018
@ncoden
Copy link
Contributor

ncoden commented Jul 4, 2018

See remix-run/history#371

@DanielRuf
Copy link
Contributor

We do not use the search param or any other user controlled value ;-)

I see no real security issue. Just a replacement with the id from the DOM.

@DanielRuf
Copy link
Contributor

Closing as we did not receive any further information.

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

No branches or pull requests

3 participants