> When an X-ray wrapper is used to access an object in another compartment, we run the C++ code without switching to the object's compartment. So whenever a C++ XPCOM method uses the JSAPI, it has to switch compartments, just in case it's being called from chrome.
It's not just chrome. It's anyone who uses Xray behavior. This includes all cross-origin access, as well as any access by sandboxes that haven't set wantXrays to false. A big consumer of this is jetpack content scripts. > bholley thinks the nub of the problem is that C++ code shouldn't use the JSAPI directly; instead it should use C++ helper classes that automatically switch compartments as needed. I disagree. This strikes me as sweeping the problem under the rug. The root of all evil here is the x-ray wrapper behavior. An object's methods should never be called in a state where GetCurrentJSContext() returns a cx that's in some other compartment. Well, it's not really that simple. The "object" referred to in the above paragraph is a C++ object, not a JS object. It generally knows nothing about its JS reflection. This is why this Xray behavior has generally worked fine for years. The problem happens when people try to find and access said reflection, usually by way of the wrapper cache. A related problem is that people are creating typed arrays with the cx compartment, stashing them (and presumably CCing them) on the C++ object, and (later) expecting them to be same-compartment with the object in the wrapper cache. This has become more of a problem recently because more DOM APIs do that sort of thing. But it's still not the common case. So I agree that the Xray behavior exacerbates this problem. But from a theoretical perspective, it's not an Xray problem. C++ code could equally end up in those methods with some differently-compartmented cx on the stack. The problem is that C++ code is assuming that the compartments of GetCurrentJSContext() and xpcomObj->GetWrapper() are guaranteed to be same-compartment, when they're not. > Part 1 - Fix bug 751926 exactly as bz originally suggested, over bholley's objections in comment 3. Make x-ray wrappers always change compartments to the wrapped object's compartment before calling its methods. My objections in comment 3 were not to the proposal you're offering. They were to bz's proposal to have two compartments per context. I still believe that is a bad approach. > > (We will have to use a hack or two to keep GetSubjectPrincipal working as it currently does, since it currently relies on js::GetContextCompartment. This doesn't sound bad to me. The status quo is much worse.) This seriously understates the significance of this problem. Indeed, this is the _entire_ problem. The reason Xrays don't enter the target compartment is so that they can execute with the privileges and context of the caller. Let me elaborate on why this is important. First, being able to pull the principal directly off the cx compartment (a long struggle that I finally got landed at the beginning of the summer) is huge. It fixed a gigantic class of security bugs that had plagued us (and kept certain security researchers busy) for years. Keeping track of the subject principal is conceptually and practically analogous to making sure we're in the right compartment. So divorcing the two just means that we're bound to get the former wrong, except that we don't get any handy assertions when that happens. They're also hard to diagnose. Any JS hacker can diagnose (and fix) a compartment mismatch. Diagnosing an incorrect principal is much harder. Moreover, I've been overhauling a lot of the security architecture to simplify security checks. One of the basic premises here is that C++ can infer its security relationship to an object being manipulated just by inspecting it. Once we know that the cx compartment (and thus principal) matches the object principal, the presence or lack of security wrappers in our "view" of the object tells us everything we need to know. This was the foundation for the 100k patch I landed removing all the cx-passing around the DOM bindings (sfink and bz were quite pleased). In general, the idea of a.com script entering b.com's compartment while retaining a.com's principal terrifies me, in both directions. Especially in the cross-origin case, I think the two domains have no business entering each others' compartments. bholley _______________________________________________ dev-tech-js-engine-internals mailing list [email protected] https://lists.mozilla.org/listinfo/dev-tech-js-engine-internals
