1426002 - Assertion failure: JS_GetCompartmentPrincipals(js::GetObjectCompartment(wrapper)) == nsJSPrincipals::get(NodePrincipal()), at /builds/worker/workspace/build/src/dom/html/nsHTMLDocument.cpp:1797

Status: RESOLVED FIXED

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

Description

[Jason Kratzer [:jkratzer]]

Attachment: trigger.html

Testcase found while fuzzing mozilla-central rev 5572465c08a9.

OS|Linux|0.0.0 Linux 4.4.0-104-generic #127-Ubuntu SMP Mon Dec 11 12:16:42 UTC 2017 x86_64
CPU|amd64|family 6 model 78 stepping 3|1
GPU|||
Crash|SIGSEGV|0x0|0
0|0|libxul.so|nsHTMLDocument::Open|hg:hg.mozilla.org/mozilla-central:dom/html/nsHTMLDocument.cpp:5572465c08a9|1795|0x0
0|1|libxul.so|nsHTMLDocument::Open|hg:hg.mozilla.org/mozilla-central:dom/html/nsHTMLDocument.cpp:5572465c08a9|1426|0x1f
0|2|libxul.so|nsHTMLDocument::WriteCommon|hg:hg.mozilla.org/mozilla-central:dom/html/nsHTMLDocument.cpp:5572465c08a9|1968|0x2
0|3|libxul.so|nsHTMLDocument::WriteCommon|hg:hg.mozilla.org/mozilla-central:dom/html/nsHTMLDocument.cpp:5572465c08a9|1888|0xd
0|4|libxul.so|mozilla::dom::HTMLDocumentBinding::writeln|s3:gecko-generated-sources:813f4744c2bbc36b7e444118a5fee2fdd0fff384cff92e8dc98cef38e6349cb79ee1f3315d9fb99146c47fb2f48a5b8461b2a8af412b2c9cfe23f5c3d1e7b921/dom/bindings/HTMLDocumentBinding.cpp:|704|0x15
0|5|libxul.so|mozilla::dom::GenericBindingMethod|hg:hg.mozilla.org/mozilla-central:dom/bindings/BindingUtils.cpp:5572465c08a9|3042|0x9
0|6|libxul.so|js::CallJSNative|hg:hg.mozilla.org/mozilla-central:js/src/jscntxtinlines.h:5572465c08a9|291|0x6
0|7|libxul.so|js::InternalCallOrConstruct|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:5572465c08a9|473|0xf
0|8|libxul.so|InternalCall|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:5572465c08a9|522|0xd
0|9|libxul.so|js::Call|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:5572465c08a9|541|0x5
0|10|libxul.so|js::ForwardingProxyHandler::call|hg:hg.mozilla.org/mozilla-central:js/src/proxy/Wrapper.cpp:5572465c08a9|176|0x9
0|11|libxul.so|js::CrossCompartmentWrapper::call|hg:hg.mozilla.org/mozilla-central:js/src/proxy/CrossCompartmentWrapper.cpp:5572465c08a9|359|0x13
0|12|libxul.so|js::Proxy::call|hg:hg.mozilla.org/mozilla-central:js/src/proxy/Proxy.cpp:5572465c08a9|511|0x15
0|13|libxul.so|js::proxy_Call|hg:hg.mozilla.org/mozilla-central:js/src/proxy/Proxy.cpp:5572465c08a9|770|0x9
0|14|libxul.so|js::CallJSNative|hg:hg.mozilla.org/mozilla-central:js/src/jscntxtinlines.h:5572465c08a9|291|0x6
0|15|libxul.so|js::InternalCallOrConstruct|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:5572465c08a9|455|0x12
0|16|libxul.so|InternalCall|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:5572465c08a9|522|0xd
0|17|libxul.so|Interpret|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:5572465c08a9|528|0xf
0|18|libxul.so|js::RunScript|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:5572465c08a9|423|0xb
0|19|libxul.so|js::ExecuteKernel|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:5572465c08a9|706|0x5
0|20|libxul.so|js::Execute|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:5572465c08a9|739|0x5
0|21|libxul.so|ExecuteScript|hg:hg.mozilla.org/mozilla-central:js/src/jsapi.cpp:5572465c08a9|4659|0x11
0|22|libxul.so|ExecuteScript|hg:hg.mozilla.org/mozilla-central:js/src/jsapi.cpp:5572465c08a9|4678|0x8
0|23|libxul.so|nsJSUtils::ExecutionContext::CompileAndExec|hg:hg.mozilla.org/mozilla-central:dom/base/nsJSUtils.cpp:5572465c08a9|266|0xc
0|24|libxul.so|mozilla::dom::ScriptLoader::EvaluateScript|hg:hg.mozilla.org/mozilla-central:dom/script/ScriptLoader.cpp:5572465c08a9|2287|0x12
0|25|libxul.so|mozilla::dom::ScriptLoader::ProcessRequest|hg:hg.mozilla.org/mozilla-central:dom/script/ScriptLoader.cpp:5572465c08a9|1929|0xb
0|26|libxul.so|mozilla::dom::ScriptLoader::ProcessScriptElement|hg:hg.mozilla.org/mozilla-central:dom/script/ScriptLoader.cpp:5572465c08a9|1627|0xf
0|27|libxul.so|mozilla::dom::ScriptElement::MaybeProcessScript|hg:hg.mozilla.org/mozilla-central:dom/script/ScriptElement.cpp:5572465c08a9|147|0x13
0|28|libxul.so|nsIScriptElement::AttemptToExecute|hg:hg.mozilla.org/mozilla-central:dom/script/nsIScriptElement.h:5572465c08a9|226|0x3
0|29|libxul.so|nsHtml5TreeOpExecutor::RunScript|hg:hg.mozilla.org/mozilla-central:parser/html/nsHtml5TreeOpExecutor.cpp:5572465c08a9|735|0x10
0|30|libxul.so|nsHtml5TreeOpExecutor::RunFlushLoop|hg:hg.mozilla.org/mozilla-central:parser/html/nsHtml5TreeOpExecutor.cpp:5572465c08a9|539|0x8
0|31|libxul.so|nsHtml5ExecutorFlusher::Run|hg:hg.mozilla.org/mozilla-central:parser/html/nsHtml5StreamParser.cpp:5572465c08a9|130|0x10
0|32|libxul.so|mozilla::SchedulerGroup::Runnable::Run|hg:hg.mozilla.org/mozilla-central:xpcom/threads/SchedulerGroup.cpp:5572465c08a9|395|0x1c
0|33|libxul.so|nsThread::ProcessNextEvent|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:5572465c08a9|1039|0x15
0|34|libxul.so|NS_ProcessNextEvent|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:5572465c08a9|508|0x11
0|35|libxul.so|mozilla::ipc::MessagePump::Run|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:5572465c08a9|97|0xa
0|36|libxul.so|MessageLoop::RunInternal|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:5572465c08a9|326|0x17
0|37|libxul.so|MessageLoop::Run|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:5572465c08a9|319|0x8
0|38|libxul.so|nsBaseAppShell::Run|hg:hg.mozilla.org/mozilla-central:widget/nsBaseAppShell.cpp:5572465c08a9|157|0xd
0|39|libxul.so|XRE_RunAppShell|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:5572465c08a9|875|0x11
0|40|libxul.so|mozilla::ipc::MessagePumpForChildProcess::Run|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:5572465c08a9|269|0x5
0|41|libxul.so|MessageLoop::RunInternal|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:5572465c08a9|326|0x17
0|42|libxul.so|MessageLoop::Run|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:5572465c08a9|319|0x8
0|43|libxul.so|XRE_InitChildProcess|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:5572465c08a9|701|0x8
0|44|firefox|content_process_main|hg:hg.mozilla.org/mozilla-central:ipc/contentproc/plugin-container.cpp:5572465c08a9|63|0x14
0|45|firefox|main|hg:hg.mozilla.org/mozilla-central:browser/app/nsBrowserApp.cpp:5572465c08a9|280|0x11
0|46|libc-2.23.so||||0x20830
0|47|firefox|MOZ_ReportAssertionFailure|hg:hg.mozilla.org/mozilla-central:mfbt/Assertions.h:5572465c08a9|165|0x5

Comment 1

[Andrew Overholt [:overholt]]

Boris, seems this has been around for a while. Can you take a quick look?

Comment 2

[Boris Zbarsky [:bzbarsky]]

I had to run on Mac (not Linux) to reproduce, and load the testcase over http.

What I see is that NodePrincipal() is a principal for http://web.mit.edu/bzbarsky/www/test.html while JS_GetCompartmentPrincipals(js::GetObjectCompartment(wrapper)) is a principal for http://web.mit.edu/bzbarsky/www/1

I haven't tracked down why that happens yet, not least because I can't reproduce on Linux and debugging on Mac is super-painful. :(

Comment 3

[Boris Zbarsky [:bzbarsky]]

OK, I reproduced under rr by disabling e10s on Linux.

Here's what happens:

1) We call o2 = window.open('1')
2) We call o2.document.writeln('') (at this point o2 contains an about:blank document)
3) The open() call implied by writeln does nsDocumentViewer::PermitUnload
4) This fires the beforeunload event, which spins the event loop using sync XHR.
5) As the event loop spins, the load of '1' completes enough to call SetNewDocument() on the window.
   Since we're an initial about:blank document and the load is same-origin we keep the same global,
   so keep the same compartment.  We just update its principal to that of the '1' document.
6) We unwind back to the nsHTMLDocument::Open call.
7) We call GetInnerWindow() at https://searchfox.org/mozilla-central/rev/ff462a7f6b4dd3c3fdc53c9bc0b328f42a5b3d2b/dom/html/nsHTMLDocument.cpp#1666 and get null.
   As a result we don't do new-window creation or wrapper reparenting.

That's basically it.  Now we have a wrapper that came from the original about:blank window, its compartment got changed to a different principal in step 5, but our document's principal is still the same as it was all along, so we fail the assertion in question.

So the simple thing to do would be to early-return if we have no inner window at this point.  We do check mScriptGlobalObject on entry to Open(), but it goes null during the beforeunload bits.

I also filed https://github.com/whatwg/html/issues/3306 to get the spec sorted out here, because it has similar problems.

I don't think there's a serious user-facing security issue here: the two principals involved are guaranteed to be same-origin.  We should still fix the invariant violation, though.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Bail out of document.open if beforeunload tears things down

I tried to add a test for this to our crashtest harness, but it did not fail there, even when loaded over HTTP

Comment 5

[Boris Zbarsky [:bzbarsky]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/42e0ec24006e1dc54b759d8727f55903a88b11e1

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/42e0ec24006e

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Calling this wontfix for backports given the severity and where we are in the cycle. Feel free to set statuses to affected and nominate for approval if you feel strongly otherwise.