Closed Bug 821760 Opened 7 years ago Closed 7 years ago

Static operations called via Xrays end up with compartment mismatches


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

Not set



Tracking Status
firefox17 --- unaffected
firefox18 --- unaffected
firefox19 + fixed
firefox20 + fixed
firefox-esr10 --- unaffected
firefox-esr17 --- unaffected


(Reporter: bzbarsky, Assigned: bzbarsky)



(Keywords: sec-critical, Whiteboard: [adv-main19-])


(1 file)

Code looks like this:

  if (js::IsWrapper(obj)) {
    obj = XPCWrapper::Unwrap(cx, obj, false);
    if (!obj) {
      return Throw<true>(cx, NS_ERROR_XPC_SECURITY_MANAGER_VETO);
    val.setObjectOrNull(JS_GetGlobalForObject(cx, obj));

with nary a compartment-enter in sight.

I just checked, and this is the only consumer of XPCWrapper::Unwrap that doesn't handle compartments in the process.
Keywords: sec-high
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Attachment #692337 - Flags: review?(peterv) → review+
Flags: in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla20
Comment on attachment 692337 [details] [diff] [review]
Properly handle compartments in WebIDL static operation bindings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 763643
User impact if declined: Some crashes with the fatal compartment checks, possible
   security issues.
Testing completed (on m-c, etc.): Passes tests.  Should have no behavior impact
Risk to taking this patch (and alternatives if risky): Should be low risk.  The
   other option is to ship the compartment mismatch, which I think is
String or UUID changes made by this patch: None.
Attachment #692337 - Flags: approval-mozilla-aurora?
Closed: 7 years ago
Resolution: --- → FIXED
There seem to be no crashes with this stack with a build of 12-16 or later. Hooray.
Comment on attachment 692337 [details] [diff] [review]
Properly handle compartments in WebIDL static operation bindings.

Hooray indeed, please go ahead with uplift.
Attachment #692337 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
As an aside, this should have had sec-approval+ per policy before going into trunk since it affected Aurora.

"Core-security bug fixes should just be landed by a developer without any explicit approval if:

    The bug has a sec-low, sec-moderate, sec-other, or sec-want rating.
    The bug is a recent regression on mozilla-central (this means that the specific regressing check-in has been identified on mozilla-central) 

This means that the developer can mark the status flags for ESR, Beta, and Aurora as 'unaffected.' It also means that we haven't shipped anywhere public in an official release yet."

I know that the last comment may seem confusing but it means that it hasn't gone out in an Aurora or Beta build yet.
Duplicate of this bug: 827962
Whiteboard: [adv-main19-]
Group: core-security
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.