Bug 16775

Summary: Security exploit in postMessage using js to override the URI and domain
Product: WebKit Reporter: Adam Barth <abarth>
Component: HTML DOMAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, collinj, darin, ian, sam
Priority: P1 Keywords: NeedsRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://crypto.stanford.edu/~abarth/research/webkit/base/
Attachments:
Description Flags
Adds integrity to postMessage URI
none
Updated patch with more tests and updated document.domain behavior sam: review+

Description Adam Barth 2008-01-07 12:26:12 PST
If an HTML document contains a <base> tag, the value of its href attribute overwrites the value of document.documentURI.  This is inconsistent with the other two browsers that implement documentURI, Firefox and Opera.  Also, JavaScript should not be able to overwrite the value of document.documentURI.

This bug has some security consequences.  For example, an attacker can set the uri parameter of postMessage to an arbitrary URI, defeating any security checks using uri.  The uri parameter is important for distinguishing between HTTP and HTTPS as well as determining whether the sender's host is different from its document.domain value.

(Not filing as security sensitive because postMessage appears to be the only WebCore consumer of this API and hasn't shipped yet.)
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" [1], 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" [2], which sounds a lot like document.documentURI.

[1] http://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/core.html#Node3-baseURI
[2] 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.
Comment 11 Sam Weinig 2008-01-20 15:25:54 PST
Filed bug 16950 as a follow up.