1764715 - Assertion failure: depth == bce_->bytecodeSection().stackDepth(), at frontend/OptionalEmitter.cpp:128 with OOM in module parsing

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20220411-87b37ed2950d (debug build, run with --fuzzing-safe --cpu-count=2 --ion-offthread-compile=off):

r(`
  while (true) {
    var line = y.shift();
    if (y.shift() == null) {
      let m94 = parseModule(undefined);
      for (var i72 = 0; i72 < 5; i72++)
        a1 = strictAssign();
    }
    a3 = class extends a93?.b34 {}.get();
  }
`);
function r(src) {
  oomTest(function() {
    let m = parseModule(src);
  });
}

Backtrace:

received signal SIGSEGV, Segmentation fault.
0x00005555574989be in js::frontend::OptionalEmitter::emitOptionalJumpTarget(JSOp, js::frontend::OptionalEmitter::Kind) ()
#0  0x00005555574989be in js::frontend::OptionalEmitter::emitOptionalJumpTarget(JSOp, js::frontend::OptionalEmitter::Kind) ()
#1  0x0000555557458ba7 in js::frontend::BytecodeEmitter::emitOptionalChain(js::frontend::UnaryNode*, js::frontend::ValueUsage) ()
#2  0x000055555743fd66 in js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*, js::frontend::ValueUsage, js::frontend::BytecodeEmitter::EmitLineNumberNote) ()
#3  0x000055555744692b in js::frontend::BytecodeEmitter::emitClass(js::frontend::ClassNode*, js::frontend::BytecodeEmitter::ClassNameKind, js::frontend::TaggedParserAtomIndex) ()
#4  0x000055555743fec7 in js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*, js::frontend::ValueUsage, js::frontend::BytecodeEmitter::EmitLineNumberNote) ()
#5  0x000055555743f49f in js::frontend::BytecodeEmitter::emitPropLHS(js::frontend::PropertyAccess*) ()
#6  0x000055555745717e in js::frontend::BytecodeEmitter::emitCalleeAndThis(js::frontend::ParseNode*, js::frontend::ParseNode*, js::frontend::CallOrNewEmitter&) ()
#7  0x00005555574581e2 in js::frontend::BytecodeEmitter::emitCallOrNew(js::frontend::CallNode*, js::frontend::ValueUsage) ()
#8  0x000055555743f91d in js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*, js::frontend::ValueUsage, js::frontend::BytecodeEmitter::EmitLineNumberNote) ()
#9  0x000055555744ac25 in js::frontend::BytecodeEmitter::emitAssignmentOrInit(js::frontend::ParseNodeKind, js::frontend::ParseNode*, js::frontend::ParseNode*) ()
#10 0x000055555743f8bb in js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*, js::frontend::ValueUsage, js::frontend::BytecodeEmitter::EmitLineNumberNote) ()
#11 0x0000555557453200 in js::frontend::BytecodeEmitter::emitExpressionStatement(js::frontend::UnaryNode*) ()
#12 0x000055555743fba5 in js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*, js::frontend::ValueUsage, js::frontend::BytecodeEmitter::EmitLineNumberNote) ()
#13 0x000055555745304f in js::frontend::BytecodeEmitter::emitStatementList(js::frontend::ListNode*) ()
#14 0x000055555743fd96 in js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*, js::frontend::ValueUsage, js::frontend::BytecodeEmitter::EmitLineNumberNote) ()
#15 0x000055555744d68e in js::frontend::BytecodeEmitter::emitLexicalScope(js::frontend::LexicalScopeNode*) ()
#16 0x000055555743fafe in js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*, js::frontend::ValueUsage, js::frontend::BytecodeEmitter::EmitLineNumberNote) ()
#17 0x0000555557450811 in js::frontend::BytecodeEmitter::emitWhile(js::frontend::BinaryNode*) ()
#18 0x000055555743fe3b in js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*, js::frontend::ValueUsage, js::frontend::BytecodeEmitter::EmitLineNumberNote) ()
#19 0x000055555745304f in js::frontend::BytecodeEmitter::emitStatementList(js::frontend::ListNode*) ()
#20 0x000055555743fd96 in js::frontend::BytecodeEmitter::emitTree(js::frontend::ParseNode*, js::frontend::ValueUsage, js::frontend::BytecodeEmitter::EmitLineNumberNote) ()
#21 0x00005555574434c2 in js::frontend::BytecodeEmitter::emitScript(js::frontend::ParseNode*) ()
#22 0x000055555746a8ee in ModuleCompiler<char16_t>::compile(JSContext*) ()
#23 0x000055555746a303 in bool ParseModuleToStencilAndMaybeInstantiate<char16_t>(JSContext*, js::frontend::CompilationInput&, JS::SourceText<char16_t>&, mozilla::Variant<mozilla::UniquePtr<js::frontend::ExtensibleCompilationStencil, JS::DeletePolicy<js::frontend::ExtensibleCompilationStencil> >, RefPtr<js::frontend::CompilationStencil>, js::frontend::CompilationGCOutput*>&) ()
#24 0x0000555557434019 in js::frontend::CompileModule(JSContext*, JS::ReadOnlyCompileOptions const&, JS::SourceText<char16_t>&) ()
#25 0x0000555556b67b3e in ParseModule(JSContext*, unsigned int, JS::Value*) ()
#26 0x00002504b4fef0df in ?? ()
[...]
#30 0x0000000000000000 in ?? ()
rax	0x5555557fb8a5	93824995014821
rbx	0x7fffffff8c30	140737488325680
rcx	0x555558207138	93825039102264
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7fffffff8c20	140737488325664
rsp	0x7fffffff8c00	140737488325632
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f99840	140737353717824
r10	0x0	0
r11	0x0	0
r12	0x1	1
r13	0x0	0
r14	0x1	1
r15	0x0	0
rip	0x5555574989be <js::frontend::OptionalEmitter::emitOptionalJumpTarget(JSOp, js::frontend::OptionalEmitter::Kind)+238>
=> 0x5555574989be <_ZN2js8frontend15OptionalEmitter22emitOptionalJumpTargetE4JSOpNS1_4KindE+238>:	movl   $0x80,0x0
   0x5555574989c9 <_ZN2js8frontend15OptionalEmitter22emitOptionalJumpTargetE4JSOpNS1_4KindE+249>:	callq  0x555556bcb517 <abort>

This is an OOM but the assertion doesn't look good and varies based on the testcase (and the fact that it requires an OOM condition doesn't necessarily make it less exploitable). Marking this s-s until investigated further.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1764715 - Add missing return for OOM handling. r?arai!

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 9272275 [details]
Bug 1764715 - Add missing return for OOM handling. r?arai!

Security Approval Request

• How easily could an exploit be constructed based on the patch?: Hard. It's an OOM issue that's likely pretty hard to trigger reliably.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: 100+ (beta)
• If not all supported branches, which bug introduced the flaw?: Bug 1761755
• Do you have backports for the affected branches?: Yes
• If not, how different, hard to create, and risky will they be?: Should apply.
• How likely is this patch to cause regressions; how much testing does it need?: Extremely unlikely.

Comment 6

[Bugmon [:jkratzer for issues]]

Bugmon Analysis
Verified bug as reproducible on mozilla-central 20220414092955-6c55ba30c858.
The bug appears to have been introduced in the following build range:

Start: b8f0d45667874418d4d040d401e9ee16cc4c8e9b (20220329090411)
End: 79f4d29fb759c9ab71e271a3cd646a90c51cd1af (20220329090547)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=b8f0d45667874418d4d040d401e9ee16cc4c8e9b&tochange=79f4d29fb759c9ab71e271a3cd646a90c51cd1af

Comment 7

[Nicolas B. Pierron [:nbp]]

S3, because this is a worrying assertion, but probably difficult to exploit given that this is a OOM handling issue.

Comment 8

[Tom Ritter [:tjr]]

Comment on attachment 9272275 [details]
Bug 1764715 - Add missing return for OOM handling. r?arai!

Approved to land and uplift

Comment 9

[Jan de Mooij [:jandem]]

Comment on attachment 9272275 [details]
Bug 1764715 - Add missing return for OOM handling. r?arai!

Beta/Release Uplift Approval Request

• User impact if declined: Crashes, maybe security issues.
• 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): Very safe patch, just adds a missing return statement to handle an out-of-memory case.
• String changes made/needed: N/A

Comment 10

[Jan de Mooij [:jandem]]

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

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Add missing return for OOM handling. r=arai
https://hg.mozilla.org/integration/autoland/rev/a0555121785b242e3f4a2accad0ad4112ccf26f4
https://hg.mozilla.org/mozilla-central/rev/a0555121785b

Comment 12

[Bugmon [:jkratzer for issues]]

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

Comment 13

[Dianna Smith [:diannaS]]

Comment on attachment 9272275 [details]
Bug 1764715 - Add missing return for OOM handling. r?arai!

Approved for 100.0b9

Comment 14

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-beta/rev/341942ae8f31