FileReader's onload event may cause 'compartment mismatched' assertion

RESOLVED FIXED in mozilla14

Status

()

Core
DOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: markh, Assigned: markh)

Tracking

Trunk
mozilla14
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mentor=khuey][lang=c++][good first bug])

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
Created attachment 608588 [details]
test case to demonstrate assertion.

I'm attaching a test case which can cause attempts to access the '.result' attribute during an onload event in a file reader to assert:

    *** Compartment mismatch 070B0788 vs. 036FD210
    Assertion failure: compartment mismatched, at ...\js\src\jscntxtinlines.h:156

In the test, the following (somewhat unusual) steps are taken:

* Create an iframe.
* Create a sandbox with the same principal as the iframe.
* Inject the FileReader function into the sandbox from the iframe.
* Execute code in the sandbox which uses the FileReader

FWIW, we also use the same technique for WebSocket and this works fine.

The test-case is a bootstrap.js for an extension.  You will also need a trivial install.rdf for this to work - I can attach one if desired.  You should see the assertion/crash at startup in a debug build.  My testing shows the onload event is delivered but the assertion happens when accessing the .result attribute of the FileReader.  The same test from "normal" content (ie, without the sandbox interaction) works correctly.  In a release build the code works correctly (ie, the .result attribute reference returns the correct value)

A snipped stack-trace from MSVC is:

	mozglue.dll!MOZ_Crash()
 	mozglue.dll!MOZ_Assert(const char * s=0x6915785c, const char * file=0x69156834, int ln=156)
 	mozjs.dll!js::CompartmentChecker::fail(JSCompartment * c1=0x0678a868, JSCompartment * c2=0x04c02068)
 	mozjs.dll!js::CompartmentChecker::check(JSCompartment * c=0x04c02068)
 	mozjs.dll!js::CompartmentChecker::check(JSObject * obj=0x05e485e0)
 	mozjs.dll!js::CompartmentChecker::check(const JS::Value & v={...})
 	mozjs.dll!js::assertSameCompartment<JSObject *,JS::Value>...
 	mozjs.dll!js::CallJSPropertyOp(...
 	mozjs.dll!js::ProxyHandler::get(...
 	xul.dll!xpc::XrayWrapper<js::SecurityWrapper<js::CrossCompartmentWrapper> >::get(...
 	mozjs.dll!js::Proxy::get(...
 	mozjs.dll!proxy_GetGeneric(...
 	mozjs.dll!JSObject::getGeneric(...
 	mozjs.dll!JSObject::getGeneric(...
 	mozjs.dll!js::GetPropertyGenericMaybeCallXML(...
 	mozjs.dll!js::GetPropertyOperation(...
 	mozjs.dll!js::Interpret(...
 	mozjs.dll!js::RunScript(...
 	mozjs.dll!js::InvokeKernel(...
 	mozjs.dll!js::Invoke(...
 	mozjs.dll!js::Invoke(...
 	mozjs.dll!JS_CallFunctionValue(...
 	xul.dll!nsXPCWrappedJSClass::CallMethod(...
 	xul.dll!nsXPCWrappedJS::CallMethod(...
 	xul.dll!PrepareAndDispatch(...
 	xul.dll!SharedStub()
 	xul.dll!nsDOMEventListenerWrapper::HandleEvent(nsIDOMEvent * aEvent=0x069a97c0)
 	xul.dll!nsEventListenerManager::HandleEventSubType(...
 	xul.dll!nsEventListenerManager::HandleEventInternal(...
 	xul.dll!nsEventListenerManager::HandleEvent(...
 	xul.dll!nsEventTargetChainItem::HandleEvent(...
 	xul.dll!nsEventTargetChainItem::HandleEventTargetChain(...
 	xul.dll!nsEventDispatcher::Dispatch(...
 	xul.dll!nsEventDispatcher::DispatchDOMEvent(...
 	xul.dll!nsDOMEventTargetHelper::DispatchDOMEvent(...
 	xul.dll!nsDOMFileReader::DispatchDOMEvent(...
 	xul.dll!mozilla::dom::FileIOObject::DispatchProgressEvent(const nsAString_internal & aType={...})
 	xul.dll!mozilla::dom::FileIOObject::OnStopRequest(...
 	xul.dll!nsBaseChannel::OnStopRequest(...
 	xul.dll!nsInputStreamPump::OnStateStop()
 	xul.dll!nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream * stream=0x0567c6f8)
 	xul.dll!nsInputStreamReadyEvent::Run()
 	xul.dll!nsThread::ProcessNextEvent(...
 	xul.dll!NS_ProcessNextEvent_P(...
 	xul.dll!mozilla::ipc::MessagePump::Run(...
 	xul.dll!MessageLoop::RunInternal()
 	xul.dll!MessageLoop::RunHandler()
 	xul.dll!MessageLoop::Run()
 	xul.dll!nsBaseAppShell::Run()
 	xul.dll!nsAppShell::Run()
 	xul.dll!nsAppStartup::Run()
 	xul.dll!XRE_main(...
 	firefox.exe!do_main(...
 	firefox.exe!NS_internal_main(...
 	firefox.exe!wmain(int argc=7, wchar_t * * argv=0x000aec18)
        ...
The branch at http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDOMFileReader.cpp#238 needs to JS_WrapValue the result (much like http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsDOMMessageEvent.cpp#106 does).
Whiteboard: [mentor=khuey][lang=c++][good first bug]
(Assignee)

Comment 2

5 years ago
Created attachment 608599 [details] [diff] [review]
JS_WrapValue the array buffer result

Awesome, thanks for the clue.  The attached patch fixes this test case and also the extension from which the test derived.
Assignee: nobody → mhammond
Attachment #608599 - Flags: review?(khuey)
Comment on attachment 608599 [details] [diff] [review]
JS_WrapValue the array buffer result

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

Awesome.

::: content/base/src/nsDOMFileReader.cpp
@@ +242,5 @@
>      } else {
>        *aResult = JSVAL_NULL;
>      }
> +    if (!JS_WrapValue(aCx, aResult))
> +      return NS_ERROR_FAILURE;

Brace your ifs please.
Attachment #608599 - Flags: review?(khuey) → review+

Comment 4

5 years ago
Try run for 01392d304470 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=01392d304470
Results (out of 114 total builds):
    success: 105
    warnings: 9
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mhammond@skippinet.com.au-01392d304470
(Assignee)

Comment 5

5 years ago
pushed to inbound - http://hg.mozilla.org/integration/mozilla-inbound/rev/dc11394d4693
Target Milestone: --- → mozilla14

Comment 6

5 years ago
Should this go to aurora or beta?
https://hg.mozilla.org/mozilla-central/rev/dc11394d4693
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.