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

History API should check origin in push/replace state for opaque origin #8948

Open
shhnjk opened this issue Feb 24, 2023 · 6 comments
Open
Labels
security/privacy There are security or privacy implications topic: history

Comments

@shhnjk
Copy link
Contributor

shhnjk commented Feb 24, 2023

While the specification note around only checking URL in push/replace state makes sense for defending attacks from documents which modified document.domain, this doesn't make sense for CSP sandbox because it is likely to host untrustworthy content which can spoof URL using History API.

Repro steps

  1. Go to https://test.shhnjk.com/csp_sandbox.php?s=allow-scripts&xss=%3Cform%3E%3Ch2%3EPassword%20Please%3C/h2%3E%3Cinput%20type=text%3E%3Cinput%20type=password%3E%3Cbutton%3Elogin%3C/button%3E%3C/form%3E%3Cscript%3Ehistory.pushState(%27%27,%27%27,%27/login%27)%3C/script%3E on Firefox.
  2. Observe that the URL changes to https://test.shhnjk.com/login.
@zcorpan zcorpan added security/privacy There are security or privacy implications topic: history labels Feb 28, 2023
@zcorpan
Copy link
Member

zcorpan commented Feb 28, 2023

cc @whatwg/security

WebKit and Chromium seem to throw in this case.

@annevk
Copy link
Member

annevk commented Feb 28, 2023

Throwing seems reasonable, but on the other hand I'm not convinced "path attacks" are a real thing.

@shhnjk
Copy link
Contributor Author

shhnjk commented Feb 28, 2023

but on the other hand I'm not convinced "path attacks" are a real thing.

You can watch this talk which explains how Dropbox depends on CSP sandbox.

@freddyb
Copy link

freddyb commented Mar 1, 2023

WebKit and Chromium seem to throw in this case.

Yes, it looks like Blink allows for fragment changes, if I am reading CanChangeToUrlForHistoryApi() correctly.

@zcorpan
Copy link
Member

zcorpan commented Mar 1, 2023

With git blame it seems the original code for equalIgnoringPathQueryAndFragment is https://codereview.chromium.org/1495013002 from 2015. @mikewest do you know if there was a webcompat reason to ignore query and fragment? Allow overriding the query for opaque origins seems like it's about as much of an issue as allow overriding the path.

@mikewest
Copy link
Member

mikewest commented Mar 1, 2023

https://codereview.chromium.org/1495013002#msg6 is a better record of what I was thinking at the time than I have in my head at the moment. :) I don't recall any compatibility concerns coming into play, and I was apparently looking to whatever HTML said 8 years ago along with spot-checks of Firefox's behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security/privacy There are security or privacy implications topic: history
Development

No branches or pull requests

5 participants