Closed Bug 1608785 Opened 1 year ago Closed 1 year ago

Assertion failure: !cx->runtime()->jitRuntime()->disallowArbitraryCode, at vm/Interpreter.cpp:394 with Debugger

Categories

(Core :: JavaScript Engine, defect, P1)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla74
Tracking Status
firefox-esr68 73+ fixed
firefox72 --- wontfix
firefox73 + fixed
firefox74 + fixed

People

(Reporter: decoder, Assigned: jandem)

Details

(5 keywords, Whiteboard: [jsbugmon:update,bisect][post-critsmash-triage][adv-main73+r][adv-esr68.5+r])

Attachments

(1 file)

The following testcase crashes on mozilla-central revision 9f55d547e196+ (build with --enable-gczeal --enable-optimize --enable-debug, run with --fuzzing-safe --no-threads):

var g92 = newGlobal({
    newCompartment: true
});
var dbg = Debugger(g92);
dbg.onNewScript = function(s3) {};
g92.eval(`
  function f96() {
    class l24 {}
    f96([, 2]);
  }
  f96();
`);

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  js::RunScript (cx=0x7ffff5f27000, state=...) at js/src/vm/Interpreter.cpp:393
#1  0x0000555556bc6e91 in js::InternalCallOrConstruct (cx=0x7ffff5f27000, args=..., construct=js::NO_CONSTRUCT, reason=(unknown: -177096704)) at js/src/vm/Interpreter.cpp:583
#2  0x0000555556bd7f79 in js::Call (cx=<optimized out>, fval=..., thisv=..., args=..., rval=..., reason=js::CallReason::Call) at js/src/vm/Interpreter.cpp:628
#3  js::Call (cx=0x7ffff5f27000, fval=..., thisObj=<optimized out>, arg0=..., rval=...) at js/src/vm/Interpreter.h:111
#4  0x000055555729d445 in js::Debugger::fireNewScript (this=0x7ffff5f57800, cx=0x7ffff5f27000, scriptReferent=...) at js/src/debugger/Debugger.cpp:2198
#5  0x000055555729f569 in js::DebugAPI::slowPathOnNewScript(JSContext*, JS::Handle<JSScript*>)::$_16::operator()(js::Debugger*) const (this=<optimized out>, dbg=<optimized out>) at js/src/debugger/Debugger.cpp:2337
#6  js::Debugger::dispatchHook<js::DebugAPI::slowPathOnNewScript(JSContext*, JS::Handle<JSScript*>)::$_15, js::DebugAPI::slowPathOnNewScript(JSContext*, JS::Handle<JSScript*>)::$_16>(JSContext*, js::DebugAPI::slowPathOnNewScript(JSContext*, JS::Handle<JSScript*>)::$_15, js::DebugAPI::slowPathOnNewScript(JSContext*, JS::Handle<JSScript*>)::$_16) (cx=<optimized out>, hookIsEnabled=..., fireHook=...) at js/src/debugger/Debugger.cpp:2265
#7  js::DebugAPI::slowPathOnNewScript (cx=0x7ffff5f27000, script=...) at js/src/debugger/Debugger.cpp:2330
#8  0x0000555556bb1281 in js::MakeDefaultConstructor (cx=0x7ffff5f27000, script=..., pc=0x7ffff5f02df2 "q", proto=...) at js/src/vm/Interpreter.cpp:324
#9  0x0000087bc8385e3d in ?? ()
#10 0x0000000000000000 in ?? ()
rax	0x555555a019b7	93824997136823
rbx	0x2	2
rcx	0x55555838c380	93825040696192
rdx	0x0	0
rsi	0x7ffff6eeb770	140737336227696
rdi	0x7ffff6eea540	140737336223040
rbp	0x7ffffffcac80	140737488137344
rsp	0x7ffffffcac30	140737488137264
r8	0x7ffff6eeb770	140737336227696
r9	0x7ffff7fe6c80	140737354034304
r10	0x58	88
r11	0x7ffff6b927a0	140737332717472
r12	0x7ffff5f27000	140737319694336
r13	0x7ffffffcad60	140737488137568
r14	0x7ffffffcacd0	140737488137424
r15	0xfffdffffffffffff	-562949953421313
rip	0x555556bb1c07 <js::RunScript(JSContext*, js::RunState&)+919>
=> 0x555556bb1c07 <js::RunScript(JSContext*, js::RunState&)+919>:	movl   $0x18a,0x0
   0x555556bb1c12 <js::RunScript(JSContext*, js::RunState&)+930>:	callq  0x555556b48be0 <abort()>

Hiding based on jandem's request. Since content can't access the debugger, this probably isn't as severe as the other variants of this bug we found.

This issue might be related to the work on Stencil.

Flags: needinfo?(tcampbell)
Priority: -- → P1

This is with the debug instrumentation patch in bug 1607443. Probably goes back to bug 1533523 at least.

Flags: needinfo?(tcampbell)
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED

I think sec-moderate makes sense because of the debugger dependency. It's a low-risk fix so we should consider backporting to beta/esr68.

https://hg.mozilla.org/integration/autoland/rev/f484fad3e273945fee8a9603a08cf0f14a83d402

Keywords: sec-moderate

Comment on attachment 9120744 [details]
Bug 1608785 - Remove MClassConstructor::getAliasSet. r?tcampbell!

Beta/Release Uplift Approval Request

  • User impact if declined: Possible JIT issues when the debugger API is used.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: No
  • 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 patch to disable a particular optimization.
  • String changes made/needed: N/A
Attachment #9120744 - Flags: approval-mozilla-esr68?
Attachment #9120744 - Flags: approval-mozilla-beta?
Group: javascript-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla74

Comment on attachment 9120744 [details]
Bug 1608785 - Remove MClassConstructor::getAliasSet. r?tcampbell!

Disables an optimization to avoid a possible sec bug. Approved for 73.0b6 and 68.5esr.

Attachment #9120744 - Flags: approval-mozilla-esr68?
Attachment #9120744 - Flags: approval-mozilla-esr68+
Attachment #9120744 - Flags: approval-mozilla-beta?
Attachment #9120744 - Flags: approval-mozilla-beta+
Flags: qe-verify-
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update,bisect][post-critsmash-triage]
Whiteboard: [jsbugmon:update,bisect][post-critsmash-triage] → [jsbugmon:update,bisect][post-critsmash-triage][adv-main73+r]
Whiteboard: [jsbugmon:update,bisect][post-critsmash-triage][adv-main73+r] → [jsbugmon:update,bisect][post-critsmash-triage][adv-main73+r][adv-esr68.5+r]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.