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

VERIFIED FIXED in Firefox 33

Status

()

defect
--
critical
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: jruderman, Assigned: bzbarsky)

Tracking

(Blocks 3 bugs, {assertion, regression, testcase})

Trunk
mozilla34
x86_64
macOS
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox33+ verified, firefox34 verified)

Details

Attachments

(3 attachments)

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

I think this is a regression from within the last day or so.
Posted file stack
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?
Assignee: nobody → bzbarsky
Blocks: 965413
Flags: needinfo?(bugs)
No better ideas. Forcing null principal sounds ok to me.
Flags: needinfo?(bugs)
same assertion failure also on http://livesport4u.com/stream3.html
Blocks: 532972
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.
Flags: needinfo?(mozilla)
(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!
Flags: needinfo?(mozilla)
[Tracking Requested - why for this release]: Regression from bug 965413, which landed in 33.
Attachment #8460750 - Flags: review?(bugs) → review+
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
Attachment #8460750 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/844a2d0f9f06
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Attachment #8460750 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: qe-verify+
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.