Closed Bug 1514050 Opened 10 months ago Closed 8 months ago

Remove wrapper recomputation on document.domain changes


(Core :: DOM: Core & HTML, enhancement, P3)




Tracking Status
firefox66 --- wontfix
firefox67 --- fixed


(Reporter: bzbarsky, Assigned: bzbarsky)




(2 files)

No description provided.
Priority: -- → P3
Once bug 1363208 is done, I think what needs to happen here is:

1) Start using the CCW that bug's patches add even for the 
   same-origin-considering-domain case of Window and Location.  That way
   the wrapper identity does not conceptually change when document.domain changes.

2) When doing a cross-compartment wrap that would result in an opaque wrapper,
   use a transparent CCW if any of the following conditions hold:

   (a) The two Realms involved are same-origin (ignoring domain).
   (b) The two Realms have both set document.domain and are same tld+1

Those conditions in (2) correspond to cases in which the two Realms may already hold on to each other's objects via transparent CCWs (due to being same-origin-considering-domain in the past) and hence need to keep getting transparent CCWs.

Once we start doing compartment-per-origin things we may be able to remove (a).
Depends on: 1471496
Blocks: 1521388
Blocks: 1408996
Depends on: 1521906

We want to use a transparent CCW if there is any pair of globals, one from each
compartment, which are, or have ever been, same origin-domain in the HTML spec
sense. This is obviously required in the "are now same origin-domain" case, and in
the "were same origin-domain" case it's required because there may be existing
transparent CCWs between the compartments and we don't want them to become
opaque due to a roundtrip through the compartment boundary.

In practice, we need to consider two cases:

  1. The two compartments started out same-origin. In this case the two
    CompartmentOriginInfos will have matching (in the Equals() sense)
    GetPrincipalIgnoringDocumentDomain(). They will also have matching SiteRef(),
    of course.

  2. The two compartments started out different-origin but then at some point two
    globals in the compartments ended up same origin-domain. That requires that
    the two globals be same TLD+1 and have both set document.domain. So in this
    case the two CompartmentOriginInfos have matching SiteRef() and both test true
    for HasChangedDocumentDomain().

We only need to worry about this for web compartments, which means that we only
need to worry about cases when security checks are symmetric
(i.e. originSubsumesTarget == targetSubsumesOrigin) and neither compartment is
forcing Xrays.

The change to test_clonewrapper.xul is because in the new setup we've already
tried handing an object across origins via chrome code, so it has a cached
(opaque) wrapper. When we set document.domain and pass the same object again,
we end up picking up the cached wrapper when we try to wrap across the
compartment boundary, so don't grant access when perhaps we should...

This does lead to a possible spec violation in the following situation:

  1. Two documents (A, B) start out same-site but different-origin.
  2. Privileged code (system or extension) puts a reference to an object from
    site A into site B. This object gets an opaque CCW.
  3. Both sites set document.domain to become same-effective-script-origin and
    then site B goes through the site A Window and the object graph hanging off it
    and gets to the object involved. It gets an opaque CCW when it should have a
    transparent CCW.

We could fix this if we kept recomputing wrappers on document.domain change and
just fixed the compartment filter used by the recomputation. But this seems
like a pretty rare situation, and not one web sites can get into without an
assist from a somewhat buggy extension or system code, so let's see whether we
can just live with it and remove the recomputation.

Blocks: 1524347
Pushed by
part 1.  Change the cross-compartment wrappers we use for web objects so we can avoid recomputing them when document.domain changes.  r=bholley
part 2.  Stop recomputing cross-compartment wrappers on document.domain changes.  r=bholley
Assignee: nobody → bzbarsky
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Created web-platform-tests PR for changes under testing/web-platform/tests

Is this something you think would be good for uplift to beta 66? Or better to let it ride with 67?

Flags: needinfo?(bzbarsky)

Is this something you think would be good for uplift to beta 66?

No. It's got a ton of risky dependencies, and doesn't offer that much of a benefit on its own. Why would we want to uplift it at all?

Flags: needinfo?(bzbarsky)
Component: DOM → DOM: Core & HTML
Regressions: 1582857
You need to log in before you can comment on or make changes to this bug.