|Summary:||Security exploit in postMessage using js to override the URI and domain|
|Product:||WebKit||Reporter:||Adam Barth <abarth>|
|Component:||HTML DOM||Assignee:||Sam Weinig <sam>|
|Severity:||Normal||CC:||ap, collinj, darin, ian, sam|
|Version:||528+ (Nightly build)|
Description Adam Barth 2008-01-07 12:26:12 PST
Comment 1 Sam Weinig 2008-01-07 19:18:49 PST
Overwriting of document.documentURI seems to be allowed by the DOM 3 Core spec (http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#Document3-documentURI). That is not say we shouldn't change our behavior to match the other browsers, though I can't imagine that we would want to unless it poses a real security or compatibility concern. That said, I am not sure our doucment.documentURI implementation is correct. For this bug, I think the fix is to not use the Document::documentURI() for postMessage, but rather use the url from the FrameLoader (or perhaps one of the other 10 url related methods off the document, uhg).
Comment 2 Adam Barth 2008-01-07 21:45:13 PST
> Overwriting of document.documentURI seems to be allowed by the DOM 3 Core spec Yep. You're right. Firefox throws a "not implemented" exception when you try to assign to documentURI, which we originally took to mean it wasn't assignable. > I am not sure our doucment.documentURI implementation is correct. I think it is not meant to be affected by the <base> tag. For example, the spec says: "the base URI is computed using first the value of the href attribute of the HTML BASE element if any, and the value of the documentURI attribute from the Document interface otherwise" , which implies that the two aren't necessarily equal. > For this bug, I think the fix is to not use the Document::documentURI() for > postMessage, but rather use the url from the FrameLoader That makes sense. The HTML5 spec should probably be clarified because it saids: "the uri attribute must be set to the URI of that document" , which sounds a lot like document.documentURI.  http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#Node3-baseURI  http://www.whatwg.org/specs/web-apps/current-work/#postmessage
Comment 3 Darin Adler 2008-01-08 09:47:01 PST
I am calling this a P1 bug, because we have to either fix it or remove postMessage to avoid creating a security problem.
Comment 4 Collin Jackson 2008-01-08 17:26:06 PST
Created attachment 18336 [details] Adds integrity to postMessage URI > For this bug, I think the fix is to not use the > Document::documentURI() for postMessage, but rather > use the url from the FrameLoader (or perhaps one of the > other 10 url related methods off the document, uhg). This patch changes postMessage to use the same URL field as document.location.
Comment 5 Darin Adler 2008-01-08 17:29:18 PST
Comment on attachment 18336 [details] Adds integrity to postMessage URI Good patch! r=me However, it does not fix this bug as described in the title. It fixes the more important security problem in postMessage.
Comment 6 Collin Jackson 2008-01-11 20:05:43 PST
Created attachment 18404 [details] Updated patch with more tests and updated document.domain behavior The HTML5 spec has been clarified to specify that the domain value should not change when the page sets document.domain. This updated patch changes the browser behavior to match the new spec. There's a new test case as well.
Comment 7 Adam Barth 2008-01-18 14:05:08 PST
It looks like this patch has been r+'ed since Jan 12 but has not been landed. Is there something we can do on our end to help?
Comment 8 Sam Weinig 2008-01-18 14:28:11 PST
Oops, that is my fault. I will land it ASAP. Sorry.
Comment 9 Sam Weinig 2008-01-20 15:19:59 PST
Fix landed in r29678.
Comment 10 Sam Weinig 2008-01-20 15:22:35 PST
I am going to close this bug and open another to track the original issue, the base tag overwrites document.documentURI.