Testcase found while fuzzing mozilla-central rev 461a9c98a535 (built with: --enable-address-sanitizer --enable-fuzzing).

Testcase can be reproduced using the following commands:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch --build 461a9c98a535 --asan --fuzzing -n firefox
$ python -m grizzly.replay ./firefox/firefox testcase.html

Assertion failure: !js::IsCrossCompartmentWrapper(target), at /js/src/jsapi.cpp:514

    =================================================================
    ==83308==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x7f06ff0eb5e4 bp 0x7ffe25ddcbf0 sp 0x7ffe25ddcbf0 T0)
    ==83308==The signal is caused by a WRITE memory access.
    ==83308==Hint: address points to the zero page.
        #0 0x7f06ff0eb5e4 in JSAutoRealm::JSAutoRealm(JSContext*, JSObject*) /js/src/jsapi.cpp:514:3
        #1 0x7f06f7f8d9ef in mozilla::dom::ReadableStreamFromAlgorithms::PullCallbackImpl(JSContext*, mozilla::dom::ReadableStreamController&, mozilla::ErrorResult&)::'lambda'(JSContext*, JS::Handle<JS::Value>, mozilla::ErrorResult&, RefPtr<mozilla::dom::ReadableStreamDefaultController> const&)::operator()(JSContext*, JS::Handle<JS::Value>, mozilla::ErrorResult&, RefPtr<mozilla::dom::ReadableStreamDefaultController> const&) const /dom/streams/ReadableStream.cpp:298:27
        #2 0x7f06f7f8d718 in CallCallback<(lambda at /dom/streams/ReadableStream.cpp:286:9), 0UL> /builds/worker/workspace/obj-build/dist/include/mozilla/dom/Promise-inl.h:206:12
        #3 0x7f06f7f8d718 in already_AddRefed<mozilla::dom::Promise> mozilla::dom::(anonymous namespace)::NativeThenHandler<mozilla::dom::ReadableStreamFromAlgorithms::PullCallbackImpl(JSContext*, mozilla::dom::ReadableStreamController&, mozilla::ErrorResult&)::'lambda'(JSContext*, JS::Handle<JS::Value>, mozilla::ErrorResult&, RefPtr<mozilla::dom::ReadableStreamDefaultController> const&), mozilla::dom::ReadableStreamFromAlgorithms::PullCallbackImpl(JSContext*, mozilla::dom::ReadableStreamController&, mozilla::ErrorResult&)::'lambda'(JSContext*, JS::Handle<JS::Value>, mozilla::ErrorResult&, RefPtr<mozilla::dom::ReadableStreamDefaultController> const&), std::tuple<RefPtr<mozilla::dom::ReadableStreamDefaultController>>, std::tuple<>>::CallCallback<mozilla::dom::ReadableStreamFromAlgorithms::PullCallbackImpl(JSContext*, mozilla::dom::ReadableStreamController&, mozilla::ErrorResult&)::'lambda'(JSContext*, JS::Handle<JS::Value>, mozilla::ErrorResult&, RefPtr<mozilla::dom::ReadableStreamDefaultController> const&)>(JSContext*, mozilla::dom::ReadableStreamFromAlgorithms::PullCallbackImpl(JSContext*, mozilla::dom::ReadableStreamController&, mozilla::ErrorResult&)::'lambda'(JSContext*, JS::Handle<JS::Value>, mozilla::ErrorResult&, RefPtr<mozilla::dom::ReadableStreamDefaultController> const&) const&, JS::Handle<JS::Value>, mozilla::ErrorResult&) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/Promise-inl.h:214:12
        #4 0x7f06f7ed4ab2 in mozilla::dom::PromiseNativeThenHandlerBase::ResolvedCallback(JSContext*, JS::Handle<JS::Value>, mozilla::ErrorResult&) /dom/promise/Promise.cpp:294:29
        #5 0x7f06f7ee3025 in mozilla::dom::(anonymous namespace)::PromiseNativeHandlerShim::ResolvedCallback(JSContext*, JS::Handle<JS::Value>, mozilla::ErrorResult&) /dom/promise/Promise.cpp:469:12
        #6 0x7f06f7ee3dbf in mozilla::dom::NativeHandlerCallback(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/obj-build/dist/include/js/CallArgs.h
        #7 0x7f06fe8dded5 in CallJSNative /js/src/vm/Interpreter.cpp:486:13
        #8 0x7f06fe8dded5 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /js/src/vm/Interpreter.cpp:580:12
        #9 0x7f06fe8dffc6 in InternalCall /js/src/vm/Interpreter.cpp:647:10
        #10 0x7f06fe8dffc6 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /js/src/vm/Interpreter.cpp:679:8
        #11 0x7f06fed38a30 in Call /js/src/vm/Interpreter.h:116:10
        #12 0x7f06fed38a30 in PromiseReactionJob(JSContext*, unsigned int, JS::Value*) /js/src/builtin/Promise.cpp:2244:10
        #13 0x7f06fe8dded5 in CallJSNative /js/src/vm/Interpreter.cpp:486:13
        #14 0x7f06fe8dded5 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /js/src/vm/Interpreter.cpp:580:12
        #15 0x7f06fe8dffc6 in InternalCall /js/src/vm/Interpreter.cpp:647:10
        #16 0x7f06fe8dffc6 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /js/src/vm/Interpreter.cpp:679:8
        #17 0x7f06fea3c6db in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /js/src/vm/CallAndConstruct.cpp:119:10
        #18 0x7f06f260cc35 in mozilla::dom::PromiseJobCallback::Call(mozilla::dom::BindingCallContext&, JS::Handle<JS::Value>, mozilla::ErrorResult&) /builds/worker/workspace/obj-build/dom/bindings/./PromiseBinding.cpp:83:8
        #19 0x7f06ed6c0c99 in Call /builds/worker/workspace/obj-build/dist/include/mozilla/dom/PromiseBinding.h:198:12
        #20 0x7f06ed6c0c99 in Call /builds/worker/workspace/obj-build/dist/include/mozilla/dom/PromiseBinding.h:211:12
        #21 0x7f06ed6c0c99 in mozilla::PromiseJobRunnable::Run(mozilla::AutoSlowOperation&) /xpcom/base/CycleCollectedJSContext.cpp:213:18
        #22 0x7f06ed696abe in mozilla::CycleCollectedJSContext::PerformMicroTaskCheckPoint(bool) /xpcom/base/CycleCollectedJSContext.cpp:676:17
        #23 0x7f06ed697d8f in mozilla::CycleCollectedJSContext::AfterProcessTask(unsigned int) /xpcom/base/CycleCollectedJSContext.cpp:463:3
        #24 0x7f06ef89ee8c in XPCJSContext::AfterProcessTask(unsigned int) /js/xpconnect/src/XPCJSContext.cpp:1490:28
        #25 0x7f06ed92fbc6 in nsThread::ProcessNextEvent(bool, bool*) /xpcom/threads/nsThread.cpp:1236:24
        #26 0x7f06ed93cdfa in NS_ProcessNextEvent(nsIThread*, bool) /xpcom/threads/nsThreadUtils.cpp:480:10
        #27 0x7f06ef5464ce in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /ipc/glue/MessagePump.cpp:85:21
        #28 0x7f06ef37093a in RunInternal /ipc/chromium/src/base/message_loop.cc:370:10
        #29 0x7f06ef37093a in RunHandler /ipc/chromium/src/base/message_loop.cc:363:3
        #30 0x7f06ef37093a in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:345:3
        #31 0x7f06f89e93f9 in nsBaseAppShell::Run() /widget/nsBaseAppShell.cpp:148:27
        #32 0x7f06fe49341e in XRE_RunAppShell() /toolkit/xre/nsEmbedFunctions.cpp:721:20
        #33 0x7f06ef37093a in RunInternal /ipc/chromium/src/base/message_loop.cc:370:10
        #34 0x7f06ef37093a in RunHandler /ipc/chromium/src/base/message_loop.cc:363:3
        #35 0x7f06ef37093a in MessageLoop::Run() /ipc/chromium/src/base/message_loop.cc:345:3
        #36 0x7f06fe4929c3 in XRE_InitChildProcess(int, char**, XREChildData const*) /toolkit/xre/nsEmbedFunctions.cpp:656:34
        #37 0x55ca6dda907c in content_process_main /browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
        #38 0x55ca6dda907c in main /browser/app/nsBrowserApp.cpp:375:18
        #39 0x7f0714e0bd8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
        #40 0x7f0714e0be3f in __libc_start_main csu/../csu/libc-start.c:392:3
        #41 0x55ca6dccd388 in _start (/home/jkratzer/builds/m-c-20231006092133-fuzzing-asan-opt/firefox+0xdc388) (BuildId: f89ff6cc5ebe0a4f26e80c94f437c6b7d369c24f)
    
    AddressSanitizer can not provide additional info.
    SUMMARY: AddressSanitizer: SEGV /js/src/jsapi.cpp:514:3 in JSAutoRealm::JSAutoRealm(JSContext*, JSObject*)
    ==83308==ABORTING

Looks like a regression from bug 1846893?

I think this is an issue since the original implementation. We probably need to unwrap the result of JS::GetIteratorObject.

Set release status flags based on info from the regressing bug 1846893

:tschuster, since you are the author of the regressor, bug 1846893, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Jan, do you know how bad of a security issue this might be? There are a few previous issues with this assertion that weren't even security bugs but maybe we could end up getting confused about realms. Thanks.

Verified bug as reproducible on mozilla-central 20231010213339-7a4c861a2d10.
The bug appears to have been introduced in the following build range:

Start: 76c2ef9d279ab62d4513c845d1c71e49aaf07d8c (20230816171817)
End: 1c8dec8815d59d488dc44109a4361dcec573ec0a (20230816185722)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=76c2ef9d279ab62d4513c845d1c71e49aaf07d8c&tochange=1c8dec8815d59d488dc44109a4361dcec573ec0a

Successfully recorded a pernosco session. A link to the pernosco session will be added here shortly.

A pernosco session for this bug can be found here.

Guessing sec-high for a compartment mis-match

(In reply to Andrew McCreight [:mccr8] from comment #5)

Jan, do you know how bad of a security issue this might be? There are a few previous issues with this assertion that weren't even security bugs but maybe we could end up getting confused about realms. Thanks.

Sorry, I didn't see this earlier. The issue is that CCWs don't have a well-defined realm, but in practice they do have a realm like all other JS objects (we just use the compartment's first realm for CCWs). So we'd enter a realm in the wrapper's compartment and then work with the CCW like normal. I don't think this is bad because all realms in a content compartment are same-origin.

Tom: you know the streams code better, does this make sense?

(In reply to Tom S [:evilpie] from comment #3)

I think this is an issue since the original implementation. We probably need to unwrap the result of JS::GetIteratorObject.

At least bugmon says it might come from bug 1846893 IIU comment 6 correctly, as Kagami suspected. Does comment 9 help to know what to do here and who is going to take it?

Oh, printPreview is a chrome-or-fuzzer-only, we'll need non-fuzzer test case for this.

https://hg.mozilla.org/mozilla-central/rev/e891ad8158b3106dceb2199ed2c74c0c2f05fa88 includes https://hg.mozilla.org/mozilla-central/rev/3fda41f614dd (from https://bugzilla.mozilla.org/show_bug.cgi?id=1846893#c28) and the fuzzer failure still happens.

(In reply to Kagami [:saschanaz] (they/them) from comment #13)

https://hg.mozilla.org/mozilla-central/rev/e891ad8158b3106dceb2199ed2c74c0c2f05fa88 includes https://hg.mozilla.org/mozilla-central/rev/3fda41f614dd (from https://bugzilla.mozilla.org/show_bug.cgi?id=1846893#c28) and the fuzzer failure still happens.

That is totally expected as that patch was never supposed to fix this bug. To fix this bug we probably have to explicitly unwrap the iterator object.

Okay. I just realized that this specific assertion was a regression by bug 1846893.

I added an JSAutoRealm to to then callback lambda here.
I think that specific JSAutoRealm might just be unnecessary, because I hope our promise machinery will put us in the right realm to work with the resolved value?

So that means we are left with two other potential JSAutoRealm calls, in PullCallbackImpl and CancelCallbackImpl. Both of these are kind of annoying. We can either unwrap, or maybe remember the current realm when calling JS::GetIteratorObject? Actually maybe we should just enter the nsIGlobalObject's realm from ReadableStream::From?

Seems like Tom is investigating per comment #15, so assigning him. Tom, feel free to assign me instead if that wasn't your intention. Thank you always for working on streams!

(In reply to Tom Schuster (MoCo) from comment #15)

So that means we are left with two other potential JSAutoRealm calls, in PullCallbackImpl and CancelCallbackImpl. Both of these are kind of annoying. We can either unwrap, or maybe remember the current realm when calling JS::GetIteratorObject? Actually maybe we should just enter the nsIGlobalObject's realm from ReadableStream::From?

I'm not familiar with this code, but remembering the global might work. We do that in a few places like CallBackObject::mCallbackGlobal for example.

Actually I think it is impossible for mIteratorRecordMaybeCrossRealm to be a wrapper. We always initialize it with the return value of JS::GetIteratorObject, which will create a new iterator record object in its current realm when called.

I am just going to attach a patch for the wrong AutoRealm in the lambda and a test.

Depends on D191846

I think we don't necessarily need to treat this as sec-high based on the analysis by Jan in comment 9.

https://hg.mozilla.org/mozilla-central/rev/0cb6c672c301
https://hg.mozilla.org/mozilla-central/rev/b7e3c840e78a

Verified bug as fixed on rev mozilla-central 20231027211343-ec7d4cb306bc.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

The patch landed in nightly and beta is affected.
:tschuster, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox120 to wontfix.

For more information, please visit BugBot documentation.

Comment on attachment 9360193 [details]
Bug 1857925 - Unnecessary JSAutoRealm. r?saschanaz

Beta/Release Uplift Approval Request

• User impact if declined: Diagnostic assert
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): One-line change that is verified by tests.
• String changes made/needed:
• Is Android affected?: Yes

Comment on attachment 9360193 [details]
Bug 1857925 - Unnecessary JSAutoRealm. r?saschanaz

Approved for 120.0b7