1947141 - (CVE-2025-3031) A bug in JIT optimization: an exception about loop operation

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect, P1)

Description

[anbu]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/132.0.0.0 Safari/537.36 Edg/132.0.0.0

Steps to reproduce:

A bug in JIT optimization, whether there are security risks or not requires further in-depth analysis.

version:commit 3166120f85161b7fe84565135b04c685da1dd34f

Build options:

/bin/sh ../../gecko-dev/js/src/configure --enable-debug --disable-optimize --disable-shared-js --disable-tests --enable-gczeal

Test case:

./js --baseline-warmup-threshold=10 --ion-warmup-threshold=50 --ion-check-range-analysis --ion-extra-checks

case1:

function opt(){
  let v0 = -128;
  do {
      const v1 = v0++;
      const v2 = String;
  } while (v0 != 1000);
  const v3 = BigInt(v0);
  const v4 = BigInt.asUintN(1000,v3);
  try {
      throw 65536;
  } catch(v5) {
  }
  do {
  } while (-2812732012n == v3);
  return v4;
}
  
let a = opt();
let b = opt();
for (let i = 0; i < 55; i++) {
  opt();
}
let c = opt();
print(a);
print(b);
print(c);

case2:

function opt(){
  let v0 = -128;
  do {
      const v1 = v0++;
      const v2 = String;
  } while (v0 != 1000);
  const v3 = BigInt(v0);
  const v4 = BigInt.asUintN(1,v3);
  do {
  } while (-2812732012n == v4);
  try {
      try {
          throw 65536;
      } catch(v5) {
      }
  } catch(v6) {
  }
  return v3;
}
  
let a = opt();
let b = opt();
for (let i = 0; i < 55; i++) {
  opt();
}
let c = opt();
print(a);
print(b);
print(c);

Actual results:

The result has changed after JIT optimization.
case1:

a = 1000
b = 1000
c = 4290672329704

case2:

a = 1000
b = 1000
c = 4290672329704

Comment 1

[Matthew Gaudet (he/him) [:mgaudet]]

Not setting sev or priority until after someone looks at it -- poking Iain because he looked at the last one :)

Comment 2

[Iain Ireland [:iain]]

Huh, this is an interesting one. I believe it's a regression from bug 1914631.

Here's a reduced testcase:

function opt() {
  let v0 = 0;
  do {
    var v1 = v0++;
  } while (v0 != 1000);

  const v3 = BigInt(v0);
  try { throw 65536; } catch {}
  return v3;
}

for (var i = 0; i < 20; i++) {
  assertEq(opt(), 1000n);
}

In the resume point for the throw, the value of v3 is a recoveredOnBailout IntPtrToBigInt. The input to that node is a real int32ToIntPtr, which in turn has the add from v0++ as an input. During lowering, we know that v0 is non-negative, so we redefine the input instead of generating a LInt32ToIntPtr node.

In RIntPtrToBigInt::recover, we try to read the input. The snapshot tells us that it's stored in a stack slot. However, that stack slot was originally allocated for v0, so it's only 32 bits, not 64 bits. We end up also reading the value of v1, which was stored in the subsequent stack slot. Note that 4290672329704 is 0x0x3e7000003e8, which is 999 << 32 | 1000.

If an attacker could tweak this testcase to leak the low 32 bits of a pointer, instead of an int32, then this might provide an information leak. I haven't succeeded at that yet, but I also haven't tried very hard. I think that would count as sec-moderate.

It's not immediately obvious to me what the fix is. I think we probably either have to teach the recovery code how to sign-extend an IntPtr from a smaller stack slot, or avoid getting into that problem in the first place. It's a little annoying that LStackSlot doesn't track any information about how big the slot is.

Comment 3

[BugBot [:suhaib / :marco]]

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

:anba, since you are the author of the regressor, bug 1914631, could you take a look?

For more information, please visit BugBot documentation.

Comment 4

[Andrew McCreight [:mccr8]]

(In reply to Iain Ireland [:iain] from comment #2)

If an attacker could tweak this testcase to leak the low 32 bits of a pointer, instead of an int32, then this might provide an information leak. I haven't succeeded at that yet, but I also haven't tried very hard. I think that would count as sec-moderate.

What about on a 32-bit system? Would that leak the entire pointer?

Comment 5

[Iain Ireland [:iain]]

On a 32-bit system, intptr_t is 32 bits, so we can't get the stack slot sizes confused. I think this bug is 64-bit only.

Comment 6

[Frederik Braun [:freddy]]

Iain, can you explain whether that stack you refer is the call stack or a stack data structure internal to the JS Engine? Do you think an attacker could place another interesting variable there, like a cross-origin object or some other object where the address is at the right offset?

It kind of seems to me like this is a "sec-high" if only the reporter would put more work into it and I want to give them the chance to do so :). But let me know if I'm wrong.

Comment 7

[André Bargull [:anba]]

Attachment: Bug 1947141: Track stack slot width. r=iain!

Comment 8

[André Bargull [:anba]]

Attachment: Bug 1947141: Support reading intptr from int32 stack slot. r=iain!

Comment 9

[André Bargull [:anba]]

Attachment: Bug 1947141: Add test case. r=iain!

Comment 10

[André Bargull [:anba]]

(In reply to Iain Ireland [:iain] from comment #2)

I think we probably either have to teach the recovery code how to sign-extend an IntPtr from a smaller stack slot, or avoid getting into that problem in the first place. It's a little annoying that LStackSlot doesn't track any information about how big the slot is.

Stealing two bits to track the slot width in LStackSlot seems possible, which we can then use to create a different RValueAllocation when the IntPtr is stored in an Int32 stack slot.

Comment 11

[Iain Ireland [:iain]]

(In reply to Frederik Braun [:freddy] from comment #6)

Iain, can you explain whether that stack you refer is the call stack or a stack data structure internal to the JS Engine? Do you think an attacker could place another interesting variable there, like a cross-origin object or some other object where the address is at the right offset?

It kind of seems to me like this is a "sec-high" if only the reporter would put more work into it and I want to give them the chance to do so :). But let me know if I'm wrong.

It's the call stack in the middle of an Ion function. The bug allows the attacker to read the low 32 bits of an adjacent spill slot. This means a value can be leaked if Ion code stores it in a register and then spills it to the stack. Roughly speaking, I think it probably needs to be represented as a MIR node to be spillable. The list of MIR types is here. Most of the time pointers are going to be the addresses of GC-allocated things, but it's hard to rule out exceptions.

The biggest limitation to exploiting this is that there's a hard constraint about only leaking the lower 32 bits.

I said "sec-moderate" because this page lists "Vulnerabilities which can provide an attacker additional information or positioning that could be used in combination with other vulnerabilities" as an example, and my understanding is that this kind of information leak is not itself exploitable, but it can be combined with other bugs to build a working exploit. If that's wrong, please let me know!

Comment 12

[Frederik Braun [:freddy]]

Thank you Iain. Your explanation convinced me, that this might be a useful stepping stone for exploits but not really leading to code execution. This leaves it to the reporter to either agree with it or demonstrate.

Btw, thanks for the report anbu! :)

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/715bc4c567e3
Track stack slot width. r=iain
https://hg.mozilla.org/integration/autoland/rev/8b368c4a91a3
Support reading intptr from int32 stack slot. r=iain

Comment 14

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

https://hg.mozilla.org/mozilla-central/rev/715bc4c567e3
https://hg.mozilla.org/mozilla-central/rev/8b368c4a91a3

Comment 17

[Tom Schuster]

Attachment: advisory.txt

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/2ad55763fa7c
https://hg.mozilla.org/integration/autoland/rev/db4e8d7a3490
Add test case. r=iain

Comment 19

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/db4e8d7a3490