Bug 16848 - SecurityOrigin::copy does not copy m_domainWasSetInDOM
Summary: SecurityOrigin::copy does not copy m_domainWasSetInDOM
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-01-11 18:19 PST by Adam Barth
Modified: 2008-04-26 19:01 PDT (History)
4 users (show)

See Also:


Attachments
Fixes SecurityOrigin::copy method to copy the security origin (2.02 KB, patch)
2008-01-11 18:21 PST, Adam Barth
darin: review-
Details | Formatted Diff | Diff
Makes SecurityOriing::copy make a deep copy of SecurityOrigin (2.00 KB, patch)
2008-01-14 17:16 PST, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2008-01-11 18:19:54 PST
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.
Comment 1 Adam Barth 2008-01-11 18:21:22 PST
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.
Comment 2 Darin Adler 2008-01-11 18:35:05 PST
Comment on attachment 18402 [details]
Fixes SecurityOrigin::copy method to copy the security origin

r=me
Comment 3 Darin Adler 2008-01-13 12:22:47 PST
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().
Comment 4 Adam Barth 2008-01-14 17:16:02 PST
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.
Comment 5 Sam Weinig 2008-04-26 19:01:42 PDT
Fixed in r32597.