1915249 - (CVE-2024-9400) Assertion failure: !hasFlags(1 << InWorklist), at jit/MIR.h:715

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: stack

oomAtAllocation(985, 4);
var a = {
  valueOf(b) {
    try {
      for (let x0 = 1, x = this; x++; ) {
        for (let y = 0; y < 1; y++) {}
        for (let y = 0; y < 1; y++) {}
        for (let y = 0; y < 1; y++) {}
        for (let y = 0; y < 1; y++) {}
        for (let y = 0; y < 1; y++) {}
        var m = -0()();
        0(...0, ...0, ...0, ...0, ...0);
        e(0,0)(function () {})();
        for (let i = 0; i < 1; i++) {
          function f() {}
          0(0);
          0(0);
          var g = 0;
          var h = 0;
          1[v] += 1;
        }
        for (let y = 0; y < 1; y++) {}
        for (let y = 0; y < 1; y++) {}
        for (let y = 0; y < 1; y++) {}
        for (let y = 0; y < 1; y++) {}
        for (let y = 0, y2 = this; y < 1; y++) {}
        for (let y = 0; y < 1; y++) {}
        var z1 = {};
      }
    } finally {
    }
  },
};
a.valueOf();

(gdb) bt
#0  js::jit::MDefinition::setInWorklist (this=0x7ffff54ba810) at /home/ubu32gx500/trees/mozilla-central/js/src/jit/MIR.h:715
#1  DepthFirstSearchUse(js::jit::MIRGenerator*, mozilla::Vector<std::pair<js::jit::MPhi*, js::InlineListIterator<js::jit::MUse> >, 16ul, js::SystemAllocPolicy>&, js::jit::MPhi*)::$_0::operator()(js::jit::MPhi*, js::InlineListIterator<js::jit::MUse>) const (phi=0x7ffff54ba810, use=..., this=<optimized out>)
    at /home/ubu32gx500/trees/mozilla-central/js/src/jit/IonAnalysis.cpp:36
#2  DepthFirstSearchUse (mir=0x7ffff53d7180, worklist=..., phi=0x7ffff54ba810) at /home/ubu32gx500/trees/mozilla-central/js/src/jit/IonAnalysis.cpp:47
#3  FlagPhiInputsAsImplicitlyUsed (mir=0x7ffff53d7180, block=0x7ffff54bac28, succ=<optimized out>, worklist=...)
    at /home/ubu32gx500/trees/mozilla-central/js/src/jit/IonAnalysis.cpp:231
#4  FlagOperandsAsImplicitlyUsedAfter (mir=0x7ffff53d7180, block=0x7ffff54bac28, firstRemoved=...)
    at /home/ubu32gx500/trees/mozilla-central/js/src/jit/IonAnalysis.cpp:306
#5  0x00005555584206a4 in FlagAllOperandsAsImplicitlyUsed (mir=0x31bf4385f8963000, mir@entry=0x7ffff53d7180, block=0xaaaaaaaaaaaaaa00)
    at /home/ubu32gx500/trees/mozilla-central/js/src/jit/IonAnalysis.cpp:340
/snip

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/b2c376db1d62
user:        André Bargull
date:        Mon Jan 22 14:29:41 2024 +0000
summary:     Bug 1843499 - Part 3: Add exception stack to finally. r=iain

Run with --fuzzing-safe --ion-eager, compile with AR=ar sh ../configure --enable-debug --enable-debug-symbols --with-ccache --enable-nspr-build --enable-ctypes --enable-gczeal --enable-rust-simd --disable-tests, tested on m-c rev d5b7f825041b.

Setting s-s just in case, as this seems to involve MIR.

Andre/Iain, is bug 1843499 a likely regressor?

Comment 1

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

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

Comment 2

[André Bargull [:anba]]

Attachment: Bug 1915249: Add more nodiscard. r=iain!

Comment 3

[André Bargull [:anba]]

Not a regression from bug 1843499. Bug 1843499 changed allocations, so oomAtAllocation(985, 4) from the test case no longer triggers before bug 1843499. The underlying issue is actually from bug 1209515. Alternatively bug 1697696 can be used as the regressor, because it enabled branch pruning for Warp.

Comment 4

[Iain Ireland [:iain]]

I don't think this is likely to be practically exploitable. Attackers don't have the ability to trigger a single OOM at a particular point in Ion compilation, and then no more OOMs after that. I'll mark it as sec-low, because if we imagine that attackers somehow got a magic OOM primitive, then they could put Ion into some weird states. At first glance the potential problems look like correctness issues, not security issues, but it's not impossible that a sufficiently motivated attacker could find something.

Comment 5

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

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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0fe326a98708
Add more nodiscard. r=iain

Comment 7

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

https://hg.mozilla.org/mozilla-central/rev/0fe326a98708

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for ESR128 approval when you get a chance. It grafts cleanly.

Comment 9

[André Bargull [:anba]]

Attachment: Bug 1915249: Add more nodiscard. r=iain!

Original Revision: https://phabricator.services.mozilla.com/D220311

Comment 10

[Phabricator Automation]

esr128 Uplift Approval Request

• User impact if declined: Possible correctness issues when Ion compiler computes on objects with unexpected state. (But Ion compilation will fail at later stage anyway, b/c this is under an OOM situation.)
• Code covered by automated testing: yes
• Fix verified in Nightly: yes
• Needs manual QE test: no
• Steps to reproduce for manual QE testing: N/A
• Risk associated with taking this patch: Low
• Explanation of risk level: Low risk b/c the patch only adds missing OOM handling.
• String changes made/needed: N/A
• Is Android affected?: yes

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for ESR128 approval when you get a chance. It grafts cleanly.

Comment 12

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr128/rev/bd879b0a791f

Comment 13

[Jason Kratzer [:jkratzer]]

Attachment: advisory.txt