1037271 - "Assertion failure: !aIsSandboxed" with error page in <iframe>

Status: VERIFIED FIXED

(Core :: DOM: Navigation, defect)

Description

[Jesse Ruderman]

Attachment: testcase (must be local)

Assertion failure: !aIsSandboxed, at content/base/src/nsContentUtils.cpp:6439

I think this is a regression from within the last day or so.

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Boris Zbarsky [:bzbarsky]]

So we're ending up in nsContentUtils::SetUpChannelOwner with a null loading principal but with aIsSandboxed=true.

If loading from a file:// URI, this happens because we end up in nsDocShell::DisplayLoadError which calls nsDocShell::LoadErrorPage, which calls InternalLoad with a null owner and the INTERNAL_LOAD_FLAGS_INHERIT_OWNER.  But this URI does not inherit security context, so we just end up with null owner.

I actually get the same thing loading from Bugzilla, but for a different reason: a failing CSP check loads about:blank via the string LoadURI overload, which likewise has no owner for the load.

This is all obviously a problem for the "every load should have a principal that started it" idea.  I could just create a nullprincipal in SetUpChannelOwner if aIsSandboxed and there is no principal passed in, I guess.  That would at least make sure to propagate the sandbox flag along.

Olli, any better idaes?

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

No better ideas. Forcing null principal sounds ok to me.

Comment 4

[Carsten Book [:Tomcat]]

same assertion failure also on http://livesport4u.com/stream3.html

Comment 5

[Boris Zbarsky [:bzbarsky]]

Christoph, do you mind if I just put up a patch for this, since we need it on 33?  Note that this will likely bitrot your patches in bug 1038756 somewhat....

If you'd prefer, I could land this Aurora-33-only, but then we need a credible plan for making sure bug 1038756 lands on 34 and fixes this issue there.

Comment 6

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Boris Zbarsky [:bz] from comment #5)
> Christoph, do you mind if I just put up a patch for this, since we need it
> on 33?  Note that this will likely bitrot your patches in bug 1038756
> somewhat....
> 
> If you'd prefer, I could land this Aurora-33-only, but then we need a
> credible plan for making sure bug 1038756 lands on 34 and fixes this issue
> there.

Please go ahead and put one up. I can merge your changes into our patches - no big deal. Thanks!

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: When loading sandboxed without a loading principal, just create a NullPrincipal instead of asserting and misbehaving

Comment 8

[Boris Zbarsky [:bzbarsky]]

[Tracking Requested - why for this release]: Regression from bug 965413, which landed in 33.

Comment 9

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/844a2d0f9f06

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8460750 [details] [diff] [review]
When loading sandboxed without a loading principal, just create a NullPrincipal instead of asserting and misbehaving

Approval Request Comment
[Feature/regressing bug #]: Bug 965413 
[User impact if declined]: Some cases that should end up sandboxed won't be.
[Describe test coverage new/current, TBPL]: Passes tbpl tests
[Risks and why]: Low risk
[String/UUID change made/needed]: None

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/844a2d0f9f06

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e5a79eaa1543

Comment 13

[Ada [:adalucinet]]

Reproduced with Nightly 33.0a1 debug build (Build ID: 20140710152348) - 'Assertion failure: !aIsSandboxed, at /builds/slave/m-cen-osx64-d-0000000000000000/build/content/base/src/nsContentUtils.cpp:6439' message is displayed in Terminal after loading the attached TC.
Verified as fixed on latest Aurora 34.0a2 and Firefox 33 beta 2 (Build ID: 20140908152100) debug builds: no assertion encountered.