Jesse Ruderman Reporter
	Description • 11 years ago

      No description provided.

	Jesse Ruderman Reporter
	Comment 1 • 11 years ago

Nightly: bp-f4e93611-eac3-46ab-839a-36c5d2131213

	Bobby Holley (:bholley)
	Comment 2 • 11 years ago

Looks potentially-related to the recent window changes...

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 3 • 11 years ago

This is exciting.

We land in nsOuterWindowProxy::get with id=="self" (called via js::CrossCompartmentWrapper::get etc).  

That eventually lands us in mozilla::dom::WindowBinding::genericCrossOriginGetter, which ends up throwing because the thisobj is not actually a window.

But genericCrossOriginGetter enters the compartment of uncheckedObj.  And throwing looks like this:

          return ThrowInvalidThis(cx, args, GetInvalidThisErrorForGetter(rv == NS_ERROR_XPC_SECURITY_MANAGER_VETO), "Window");

and ThrowInvalidThis will end up doing:

  JS::Rooted<JSFunction*> func(aCx, JS_ValueToFunction(aCx, aArgs.calleev()));

which is where the compartment check fails.

We need to either reenter the compartment of "obj" before doing ThrowInvalidThis or scope the entering of the compartment of uncheckedObj more narrowly.  In fact, the only reason we need that compartment-entering behavior is for the UnwrapArg<nsGlobalWindow> call.  So maybe we can just scope the autocompartment around that?

	Bobby Holley (:bholley)
	Comment 4 • 11 years ago

(In reply to Boris Zbarsky [:bz] from comment #3)
> In fact, the only reason we need that compartment-entering
> behavior is for the UnwrapArg<nsGlobalWindow> call.  So maybe we can just
> scope the autocompartment around that?

Yes, this sounds like the right thing.

	Bobby Holley (:bholley)
	Comment 7 • 11 years ago

Comment on attachment 8347365 [details] [diff] [review]
Only enter the uncheckedObj compartment in a crossOriginGetter/Setter/Method around the UnwrapArg call that needs us to be inthat compartment.

Review of attachment 8347365 [details] [diff] [review]:
-----------------------------------------------------------------

The generated code looks right, but I'm not up to speed enough on the Codegen to review what look like nontrivial changes. bouncing that part to peter.

	Andrew McCreight [:mccr8]
	Comment 8 • 11 years ago

Setting sec-high for compartment mismatch, though I'm not sure how possible it would be to actually exploit this.  Feel free to adjust.

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 9 • 11 years ago

I don't think it's possible to exploit this, fwiw.

	Peter Van der Beken [:peterv]
	Comment 10 • 11 years ago

Comment on attachment 8347365 [details] [diff] [review]
Only enter the uncheckedObj compartment in a crossOriginGetter/Setter/Method around the UnwrapArg call that needs us to be inthat compartment.

Review of attachment 8347365 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bindings/Codegen.py
@@ +2663,5 @@
>  ${codeOnFailure}
>    }
>  }""").substitute(self.substitution, codeOnFailure=codeOnFailure)
>  
> +    def getXPConnectUnwrap(self, allowCrossOriginObj):

Not sure why this needs to be a method, might be better to just store this in a string where you define self.substitution["uncheckedObjDecl"].

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 12 • 11 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/88f21d4b4c78

	Boris Zbarsky [:bzbarsky] Assignee
	Comment 13 • 11 years ago

Comment on attachment 8349031 [details] [diff] [review]
Updated to review comments

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 946067 
User impact if declined: Can't land fix for bug 946067 on aurora, get web compat
   regressions.
Testing completed (on m-c, etc.): Passes tests
Risk to taking this patch (and alternatives if risky): Low risk, I believe.
String or IDL/UUID changes made by this patch: None

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 14 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/88f21d4b4c78

	Ryan VanderMeulen [:RyanVM]
	Comment 15 • 11 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/62d366143e50

	Matt Wobensmith [:mwobensmith][:matt:]
	Comment 16 • 11 years ago

Confirmed crash, compartment mismatch in FF29, 2013-12-13.
Verified fixed in FF28, FF29, 2014-01-18.