Closed Bug 634834 Opened 9 years ago Closed 9 years ago
It's possible to call history
.push State/replace State on a cross-origin page
The fix for bug 619359 can be circumvented. After the security check passed, when stringifying a state data argument, a new document can be loaded in the associated window.
This tries to get cookies for html5demos.com. This works on trunk.
blocking2.0: --- → ?
sg:crit? if not, doesn't block
Security bug in a new feature, blocking. Assigning to blake as wrappers should save us.
Assignee: nobody → mrbkap
blocking2.0: ? → final+
mrbkap and I don't think this is a wrappers problem. I think I have a simple fix; I'll be able to test it out in a few days.
Assignee: mrbkap → justin.lebar+bug
Justin: Do you think the fix will be risky and need beta coverage? Or would putting it just in the RC be enough. If it's just touching code run by pushState then I'd think the latter.
I think it's just a few lines in docshell, to make sure that mCurrentURI after the stringification is same-origin as the new URI.
You can't just test mCurrentURI, because AIUI that doesn't get updated until after the page gets to its destination. At least, it doesn't get updated early enough for my test. Testing that the origins of both mCurrentURI and mLoadingURI don't change as a result of the stringify seems to work. Instead of a same-origin check, I could test that mCurrentURI and mLoadingURI are the same before and after the stringification. Any thoughts on which might be better?
(In reply to comment #8) > You can't just test mCurrentURI, because AIUI that doesn't get updated until > after the page gets to its destination. At least, it doesn't get updated early > enough for my test. Oh, actually, I think my testcase is broken. mCurrentURI is good enough for the testcase in this bug.
Verified that the testcase in this bug fails with a security error.
Comment on attachment 513402 [details] [diff] [review] Patch v1 Sholdn't we be comparing principals instead?
Attachment #513518 - Flags: review?(bzbarsky)
Comment on attachment 513518 [details] [diff] [review] Patch v2 If you have an nsIDocument, just GetPrincipal on it you don't need the global obj. Other than that, looks fine.
Attachment #513518 - Flags: review?(bzbarsky) → review+
(In reply to comment #13) > If you have an nsIDocument, just GetPrincipal on it you don't need the global > obj. Really? Maybe I'm missing some special sauce, but > nsCOMPtr<nsIDocument> origDocument = do_GetInterface(GetAsSupports(this)); > nsCOMPtr<nsIPrincipal> origPrincipal = origDocument->GetPrincipal(); does not compile.
Er, GetNodePrincipal() on the document.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.