Closed Bug 634834 Opened 14 years ago Closed 14 years ago

It's possible to call history.pushState/replaceState on a cross-origin page

Categories

(Core :: Security, defect)

Product:
Core
Component:
Security
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox5 --- unaffected
firefox6 --- unaffected
firefox7 --- unaffected
firefox8 --- unaffected
blocking2.0 --- final+
status1.9.2 --- unaffected
status1.9.1 --- unaffected

People

(Reporter: moz_bug_r_a4, Assigned: justin.lebar+bug)

References

Details

(Whiteboard: [sg:high][hardblocker])

Attachments

(1 file, 1 obsolete file)

Patch v2
(deleted), patch
bzbarsky
: review+
Details | Diff | Splinter Review
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.
Attached file (deleted) —
This tries to get cookies for html5demos.com. This works on trunk.
Funtimes.
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+
Whiteboard: [sg:high]
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.
Attached patch Patch v1 (obsolete) (deleted) — — Splinter Review
Verified that the testcase in this bug fails with a security error.
Attachment #513402 - Flags: review?(bzbarsky)
Comment on attachment 513402 [details] [diff] [review] Patch v1 Sholdn't we be comparing principals instead?
Whiteboard: [sg:high] → [sg:high][hardblocker]
Attached patch Patch v2 (deleted) — — Splinter Review
Like so?
Attachment #513518 - Flags: review?(bzbarsky)
Attachment #513402 - Attachment is obsolete: true
Attachment #513402 - 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.
http://hg.mozilla.org/mozilla-central/rev/b5fdead607d4
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Blocks: 500328
Attachment #513047 - Attachment is private: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: