Closed Bug 1426002 Opened 2 years ago Closed 2 years ago

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

Categories

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

59 Branch
defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox-esr52 --- wontfix
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- fixed

People

(Reporter: jkratzer, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, sec-low, testcase, Whiteboard: [post-critsmash-triage][adv-main59+])

Attachments

(2 files)

Attached file 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
Flags: in-testsuite?
Group: core-security → dom-core-security
Boris, seems this has been around for a while. Can you take a quick look?
Flags: needinfo?(bzbarsky)
Priority: -- → P1
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. :(
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.
Assignee: nobody → bzbarsky
Flags: needinfo?(bzbarsky)
Keywords: sec-low
I tried to add a test for this to our crashtest harness, but it did not fail there, even when loaded over HTTP
Attachment #8937910 - Flags: review?(nika)
Attachment #8937910 - Flags: review?(nika) → review+
https://hg.mozilla.org/mozilla-central/rev/42e0ec24006e
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
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.
Group: dom-core-security → core-security-release
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main59+]
Group: core-security-release
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.