Bug 16848 – SecurityOrigin::copy does not copy m_domainWasSetInDOM

• Component: Platform
• Status: RESOLVED FIXED

[Adam Barth]

The new SecurityOrigin::copy method does not copy m_domainWasSetInDOM when making a copy of the security origin.  This does not appear to be exploitable currently, but could lead to two classes of bugs:

1) A document sets its document.domain and then tries to access an object that uses a copy() of its SecurityOrigin.  It can not access the document because that document's origin has forgotten that it had set its domain property.

2) A malicious document from foo.example.com could set its document.domain property to example.com, then transfer control to an object that uses a copy() of its security origin.  Once there, it could script example.com.

The copy() method also does not copy m_noAccess, which could lead to similar attacks using data: URLs.

[Adam Barth]

Created attachment 18402 [details]
Fixes SecurityOrigin::copy method to copy the security origin

This patch fixes the SecurityOrigin::copy method to copy the security origin.

[Darin Adler]

Comment on attachment 18402 [details]
Fixes SecurityOrigin::copy method to copy the security origin

r=me

[Darin Adler]

Comment on attachment 18402 [details]
Fixes SecurityOrigin::copy method to copy the security origin

After further scrutiny, there's another aspect of this patch that I'm not sure about.

This removes the copy() calls on the protocol and host. Under normal circumstances, copy() is not needed to copy a WebCore::String, but if we intend to use the string on another thread, we do need a copy(). I'm not sure it's safe to remove the copy() here, because I suspect that the caller does use this on another thread.

We either need to prove those copies aren't needed, or create a new version of this patch that retains the calls to copy().

[Adam Barth]

Created attachment 18450 [details]
Makes SecurityOriing::copy make a deep copy of SecurityOrigin

Here's an updated version of the patch that makes a deep copy.  I don't know if the SecurityOrigin::copy method is needed, but it's probably a good idea to actually have it make a copy or remove it.

[Sam Weinig]

Fixed in r32597.