925029 - compartment mismatch with HTMLDocumentBinding::DOMProxyHandler::get calling JS_HasPropertyById

Status: RESOLVED INCOMPLETE

(Core :: DOM: Core & HTML, defect)

Description

[Andrew McCreight [:mccr8]]

I see four crashes that look like this on crash-stats:
https://crash-stats.mozilla.com/report/index/c2b548a6-507d-487d-847d-795ef2131007
https://crash-stats.mozilla.com/report/index/301c99eb-7862-45e7-9e72-57aa32131004
https://crash-stats.mozilla.com/report/index/55944f65-7a6d-483f-b9e0-dd6c92131002
https://crash-stats.mozilla.com/report/index/a0fb8c90-3d87-4861-817b-fcc272131002

It looks like it must be coming from this chunk of get():
  JS::Rooted<JSObject*> expando(cx, DOMProxyHandler::GetExpandoObject(proxy));
  if (expando) {
    bool hasProp;
    if (!JS_HasPropertyById(cx, expando, id, &hasProp)) {
      return false;
    }

It isn't clear to me what on the call stack is supposed to guarantee that we've entered the compartment of the expando.

It looks like by this point we've already called JS_AlreadyHasOwnPropertyById on unforgeableHolder, which does a compartment check, so we're not totally out to lunch.

Comment 1

[Boris Zbarsky [:bzbarsky]]

So this is odd.  We're just coming from the normal call path (as expected), not via xrays, so cx should be in the compartment of proxy....

And in ReparentWrapper we do reparent the expando object, so it should be tracking the compartment of the proxy.  So there should be no need to enter any compartments; there should only be one compartment involved, no?

Comment 2

[Andrew McCreight [:mccr8]]

Ah, I see.  I guess I'm just too used to looking at Xray-related compartment mismatches.  JS_HasPropertyById is the top frame in the bulk of these compartment mismatches, so maybe the problem just lies elsewhere.  I'll mark this as sec-audit until we have something more concrete to go on...

Comment 3

[Andrew McCreight [:mccr8]]

This is pretty old and mysterious so I'm just going to close it for now.