Closed Bug 2013569 Opened 8 months ago Closed 8 months ago

Hit MOZ_CRASH(*** Compartment mismatch 5c8e5aab0c10 vs. 5c8e5aa518a0 at argument 2) at /js/src/vm/JSContext-inl.h:56 with ShadowRealm importValue

Categories

(Core :: JavaScript Engine, defect, P1)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
149 Branch
Tracking Status
firefox-esr115 --- disabled
firefox-esr140 --- disabled
firefox147 --- disabled
firefox148 --- disabled
firefox149 --- fixed

People

(Reporter: decoder, Assigned: jonco)

References

(Regression)

Details

(7 keywords, Whiteboard: [bugmon:update,bisected,confirmed])

Attachments

(4 files)

The following testcase crashes on mozilla-central revision 20260130-20b1a1e6c530 (fuzzing-opt build, run with --fuzzing-safe --no-threads):

See attachment.

Backtrace:

==1748181==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x000000000000 (pc 0x5c8e1a3feb2d bp 0x7fff5653b850 sp 0x7fff5653b840 T1748181)
==1748181==The signal is caused by a WRITE memory access.
==1748181==Hint: address points to the zero page.
    #0 0x5c8e1a3feb2d in MOZ_CrashSequence(void*, long) /builds/worker/workspace/obj-spider/dist/include/mozilla/Assertions.h:237:3
    #1 0x5c8e1a3feb2d in MOZ_Crash(char const*, int, char const*) /builds/worker/workspace/obj-spider/dist/include/mozilla/Assertions.h:375:3
    #2 0x5c8e1a3feb2d in js::ContextChecks::fail(JS::Compartment*, JS::Compartment*, int) /js/src/vm/JSContext-inl.h:55:5
    #3 0x5c8e1a3feb2d in js::ContextChecks::check(JS::Compartment*, int) /js/src/vm/JSContext-inl.h:71:7
    #4 0x5c8e1a3feb2d in js::ContextChecks::check(JSObject*, int) /js/src/vm/JSContext-inl.h:84:7
    #5 0x5c8e1a3feb2d in js::ContextChecks::check(JS::Value const&, int) /js/src/vm/JSContext-inl.h:134:7
    #6 0x5c8e1b01237a in void JSContext::checkImpl<JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>>(JS::Handle<JSObject*> const&, JS::Handle<JS::PropertyKey> const&, JS::Handle<JS::Value> const&) /js/src/vm/JSContext-inl.h:216:33
    #7 0x5c8e1b01237a in void JSContext::check<JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>>(JS::Handle<JSObject*> const&, JS::Handle<JS::PropertyKey> const&, JS::Handle<JS::Value> const&) /js/src/vm/JSContext-inl.h:223:5
    #8 0x5c8e1b01237a in DefineDataPropertyById(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::Value>, unsigned int) /js/src/vm/PropertyAndElement.cpp:74:7
    #9 0x5c8e1b012a78 in DefineDataProperty(JSContext*, JS::Handle<JSObject*>, char const*, JS::Handle<JS::Value>, unsigned int) /js/src/vm/PropertyAndElement.cpp:232:10
    #10 0x5c8e1b012a78 in JS_DefineProperty(JSContext*, JS::Handle<JSObject*>, char const*, JS::Handle<JS::Value>, unsigned int) /js/src/vm/PropertyAndElement.cpp:239:10
    #11 0x5c8e1a34ba1e in js::shell::ModuleLoader::dynamicImport(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::Handle<JS::Value>) /js/src/shell/ModuleLoader.cpp:299:8
    #12 0x5c8e1b1b6c0d in js::HostLoadImportedModule(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, unsigned int, JS::ColumnNumberOneOrigin) /js/src/vm/Modules.cpp:755:13
    #13 0x5c8e1a4c0657 in ShadowRealmImportValue(JSContext*, JS::Handle<JSString*>, JS::Handle<JSString*>, JS::Realm*, JS::Realm*) /js/src/builtin/ShadowRealm.cpp:445:10
    #14 0x5c8e1a4c0657 in ShadowRealm_importValue(JSContext*, unsigned int, JS::Value*) /js/src/builtin/ShadowRealm.cpp:621:19
    #15 0x5c8e1afa5b32 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /js/src/vm/Interpreter.cpp:490:13
    #16 0x5c8e1afa5b32 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /js/src/vm/Interpreter.cpp:586:12
    #17 0x5c8e1afa6298 in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) /js/src/vm/Interpreter.cpp:653:10
    #18 0x5c8e1afa6298 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:685:8
    #19 0x5c8e1b1601d2 in js::fun_apply(JSContext*, unsigned int, JS::Value*) /js/src/vm/JSFunction.cpp:1157:10
    #20 0x5c8e1afa5b32 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /js/src/vm/Interpreter.cpp:490:13
    #21 0x5c8e1afa5b32 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /js/src/vm/Interpreter.cpp:586:12
    #22 0x5c8e1afb0bad in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) /js/src/vm/Interpreter.cpp:653:10
    #23 0x5c8e1afb0bad in js::CallFromStack(JSContext*, JS::CallArgs const&, js::CallReason) /js/src/vm/Interpreter.cpp:658:10
    #24 0x5c8e1afb0bad in js::Interpret(JSContext*, js::RunState&) /js/src/vm/Interpreter.cpp:3272:16
    #25 0x5c8e1afa5736 in MaybeEnterInterpreterTrampoline(JSContext*, js::RunState&) /js/src/vm/Interpreter.cpp:384:10
    #26 0x5c8e1afa5736 in js::RunScript(JSContext*, js::RunState&) /js/src/vm/Interpreter.cpp:460:13
    #27 0x5c8e1afa6c82 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, js::AbstractFramePtr, JS::MutableHandle<JS::Value>) /js/src/vm/Interpreter.cpp:850:10
    #28 0x5c8e1afa6c82 in js::Execute(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>) /js/src/vm/Interpreter.cpp:880:10
    #29 0x5c8e1b1050cc in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) /js/src/vm/CompilationAndEvaluation.cpp:572:10
    #30 0x5c8e1a38ca3b in RunFile(JSContext*, char const*, _IO_FILE*, CompileUtf8, bool, bool) /js/src/shell/js.cpp:1387:10
    #31 0x5c8e1a38c570 in Process(JSContext*, char const*, bool, FileKind) /js/src/shell/js.cpp
    #32 0x5c8e1a35b3ec in ProcessArgs(JSContext*, js::cli::OptionParser*) /js/src/shell/js.cpp:12110:10
    #33 0x5c8e1a35b3ec in Shell(JSContext*, js::cli::OptionParser*) /js/src/shell/js.cpp:12363:12
    #34 0x5c8e1a351876 in main /js/src/shell/js.cpp:12769:12
    #35 0x7db8a2e2a1c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #36 0x7db8a2e2a28a in __libc_start_main csu/../csu/libc-start.c:360:3
    #37 0x5c8e1a31def8 in _start (/home/decoder/Mozilla/Anthropic/.builds/release/js/dist/bin/js+0x1fe9ef8) (BuildId: e3130fc1030f7cc3946e6339e15c506bc60c7045)

==1748181==Register values:
rax = 0x0000000000000038  rbx = 0x00005c8e1cb7b4a0  rcx = 0x0000000000000000  rdx = 0x00007db8a3004563  
rdi = 0x00007db8a3005700  rsi = 0x0000000000000000  rbp = 0x00007fff5653b850  rsp = 0x00007fff5653b840  
 r8 = 0x0000000000000000   r9 = 0x0000000000000003  r10 = 0x0000000000000002  r11 = 0x0000000000000293  
r12 = 0x00007fff5653b9b0  r13 = 0x00005c8e5aa29630  r14 = 0x0000000000000000  r15 = 0x00007fff5653b8c8  
UndefinedBehaviorSanitizer can not provide additional info.
SUMMARY: UndefinedBehaviorSanitizer: SEGV /builds/worker/workspace/obj-spider/dist/include/mozilla/Assertions.h:237:3 in MOZ_CrashSequence(void*, long)
==1748181==ABORTING

This bug is part of a series of bugs reported by Anthropic, thank you for your contributions to Firefox!

Attached file Testcase —

This enables and then uses shadow realms:

// Enable ShadowRealm feature
setPrefValue("experimental.shadow_realms", true);

We're not shipping this feature so likely not sec-sensitive. Matt is the expert on shadow realms.

Flags: needinfo?(mgaudet)

Verified bug as reproducible on mozilla-central 20260130092147-20b1a1e6c530.
Unable to bisect testcase (Testcase reproduces on start build!):

Start: d0c1a4b1cb725aaff109054e060729eb71099baf (20251102212929)
End: 20b1a1e6c530f3506d5378bf5f4da504b6f03661 (20260130092147)
BuildFlags: BuildFlags(asan=True, tsan=False, debug=False, fuzzing=True, coverage=False, valgrind=False, no_opt=False, fuzzilli=False, nyx=False, searchfox=False, afl=False)

Whiteboard: [bugmon:update,bisect] → [bugmon:update,bisected,confirmed]

I'd like to get another pair of eyes on this because it starts to wander quite quickly into module code that I don't have a lot of comfort with. I'm reasonably confident at the moment that this is not possible to trigger outside of ShadowRealms.

The basic thrust of the issue is as follows.

  • When ShadowRealms are enabled, the ShadowRealm prototype provides the importValue function. This ends up being a scripted entry point to the module loader, functionally dynamic import.
  • Because it's a scripted entry point, we can invoke it cross-compartment
  • Unfortunately, it does seem like this ends up starting to violate some assumptions inside the module loader. For example,
    1. ModuleLoader::dynamicImport assumes payload is same compartment.
    2. FinishLoadingImportedModuleFailed also assumes payload is same compartment and not a wrapper, and can't handle resolving cross realm.

I hacked together a fairly gross patch which stops the bug, but it's not something I'd like to land (not the least of which because I didn't investigate the case where module loading -succeeds-.

I'm reasonably confident this isn't triggerable from non-ShadowRealms code because dynamicImport is syntactic and can't be invoked with a different-realm-this.

Side Notes:

  1. I would not be surprised if there's a specification issue here with ShadowRealms too.
  2. We've talked idly about it perhaps being time to remove ShadowRealms. I'd totally support doing so as part of the mitigation of this bug but I'd like to check in with Dan minor before we do that. We should perhaps see if there's a smaller-blast-radius fix possible before doing that.
  3. I'm gently surprised that other fuzzing never found this one. Mostly because scanning through the importValue code it doesn't look particularly realm safe; E.g right away it seems like ShadowRealmImportValue should have wrapped promise allocated in one realm before calling into js:: HostLostImportedModule.
Flags: needinfo?(mgaudet)

Yoshi: Can you weigh in a bit on the expectations around compartments for module loading?

Flags: needinfo?(allstars.chh)

Just to race ahead of the Phabricator automation: https://phabricator.services.mozilla.com/D281265

Redirecting ni? to Jon, as apparently Yoshi is out next week :)

Flags: needinfo?(allstars.chh) → needinfo?(jcoppeard)

We're not shipping this feature so likely not sec-sensitive.

not ever? If we plan to ship it sometime it's still security sensitive, just not urgent.

If we're not close to shipping shadow realms then we should definitely remove it from the fuzzing-safe set so we stop getting bounty bugs filed against it.

Making the compartment mismatch checks be release-asserts would stop it from being a security problem, too.

I'll fix this by making it crash in release builds. We can then take the decision to make modules code supported wrapped promises or remove shadow realms later.

Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)
Attached file (secure) —

There are two problems shown up by this bug: shadow realms doesn't correctly
wrap promises it passes to the module system from other compartments, and the
module system doesn't support cross compartment promises.

This patch corrects the first problem but does not add support to the module
system. Instead it adds a release assertion and a TODO comment. This turns the
failures into a hard assert. We can implement support in the module system for
this later if we decide it's worth it.

This also adds a few other compartment checking assertions to the modules code
that were missing.

Severity: -- → S2
Priority: -- → P1
Regressed by: 1767525

Comment on attachment 9542692 [details]
(secure)

Security Approval Request

  • How easily could an exploit be constructed based on the patch?: Not hard, but user also has to enable shadow realms pref which is off by default.
  • Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: Yes
  • Which branches (beta, release, and/or ESR) are affected by this flaw, and do the release status flags reflect this affected/unaffected state correctly?: Everything back to bug 1747059
  • If not all supported branches, which bug introduced the flaw?: Bug 1747059
  • Do you have backports for the affected branches?: Yes
  • If not, how different, hard to create, and risky will they be?: Should be trivial
  • How likely is this patch to cause regressions; how much testing does it need?: Unlikely, it mostly adds release assertions.
  • Is the patch ready to land after security approval is given?: Yes
  • Is Android affected?: Yes
Attachment #9542692 - Flags: sec-approval?

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

Regressed by: 1747059
Whiteboard: [bugmon:update,bisected,confirmed] → [bugmon:update,bisected,confirmed][reminder-test 2026-04-07]

Comment on attachment 9542692 [details]
(secure)

sec-approval+, a=dveditz

Attachment #9542692 - Flags: sec-approval? → sec-approval+
Pushed by jcoppeard@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/469d1e8325e3 https://hg.mozilla.org/integration/autoland/rev/8f7ea74bade2 Make shadow realm importValue wrap promise passed to module system and assert against this r=spidermonkey-reviewers,mgaudet
Group: javascript-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → 149 Branch

Bug marked as FIXED but still reproduces on mozilla-central 20260206213744-17c590744962. If you believe this to be incorrect, please remove the bugmon keyword to prevent further analysis.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

The fix makes the testcase result in a safe crash. Please reopen if that is not what is observed.

Status: REOPENED → RESOLVED
Closed: 8 months ago → 8 months ago
Keywords: bugmon
Resolution: --- → FIXED
QA Whiteboard: [qa-triage-done-c150/b149]
QA Whiteboard: [qa-triage-done-c150/b149] → [sec] [qa-triage-done-c150/b149]

2 months ago, dveditz placed a reminder on the bug using the whiteboard tag [reminder-test 2026-04-07] .

jonco, please refer to the original comment to better understand the reason for the reminder.

Flags: needinfo?(jcoppeard)
Whiteboard: [bugmon:update,bisected,confirmed][reminder-test 2026-04-07] → [bugmon:update,bisected,confirmed]

Shadow realms have been removed in bug 2014919 so test is not applicable.

Flags: needinfo?(jcoppeard)
Group: core-security-release
Keywords: keep-hidden
See Also: → 2079067
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: