Closed Bug 1805881 Opened 1 year ago Closed 1 year ago

Assertion failure: 2 <= base && base <= 36 (base must be in range [2, 36]), at js/src/jit/MacroAssembler.cpp:1405

Categories

(Core :: JavaScript Engine, task, P1)

task

Tracking

()

RESOLVED FIXED
110 Branch
Tracking Status
firefox110 --- fixed

People

(Reporter: saelo, Assigned: anba)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

The following sample triggers an assertion failure in debug builds of Spidermonkey from latest HEAD (git commit b4de9fa6fe3fb425129093c3deb1dae0686f7a2b)
(the sample will run into a floating point exception due to a division by zero in a release build):

// Run with --baseline-warmup-threshold=10 --ion-warmup-threshold=100
function main() {
for (let v0 = 0; v0 < 100; v0++) {
    try {
        const v3 = (256)["toString"](v0);
        for (let v4 = 0; v4 < 100; v4++) {
        }
        const v5 = v3.localeCompare();
    } catch(v6) {
    }
    let v7 = 0;
    while (v7 < 6) {
        const v9 = v7++;
    }
}
// CRASH INFO
// ==========
// TERMSIG: 11
// STDERR:
// Assertion failure: 2 <= base && base <= 36 (base must be in range [2, 36]), at /home/builder/firefox/js/src/jit/MacroAssembler.cpp:1405
// #01: ???[./spidermonkey/js +0x2d2330a]
// #02: ???[./spidermonkey/js +0x2b11b5c]
// #03: ???[./spidermonkey/js +0x2aeed6e]
// #04: ???[./spidermonkey/js +0x2b5865f]
// #05: ???[./spidermonkey/js +0x2ba9322]
// #06: ???[./spidermonkey/js +0x2be86da]
// #07: ???[./spidermonkey/js +0x1c09f76]
// #08: ???[./spidermonkey/js +0x1c09ba2]
// #09: ???[./spidermonkey/js +0x1c29170]
// #10: ???[./spidermonkey/js +0x1c28d58]
// #11: ???[./spidermonkey/js +0x1c3dab4]
// #12: ???[/lib/x86_64-linux-gnu/libc.so.6 +0x94b43]
// #13: ???[/lib/x86_64-linux-gnu/libc.so.6 +0x126a00]
// #14: ??? (???:???)
// STDOUT:
// ARGS: ./spidermonkey/js --baseline-warmup-threshold=10 --ion-warmup-threshold=100
// EXECUTION TIME: 52 ms
}
main();

This will fail in js::jit::MacroAssembler::loadInt32ToStringWithBase because the value of base is zero, but is expected to be between 2 and 36. The value of the base seems controllable, for example, the following slightly modified sample will cause a base of 1 to be used (and other, seemingly arbitrarily large bases can be used as well):

function main() {
let repeat = true;
for (let v0 = 1; v0 < 100; v0++) {
    try {
        const v3 = (256)["toString"](v0);
        print(v3);
        for (let v4 = 0; v4 < 100; v4++) {
        }
        const v5 = v3.localeCompare();
    } catch(v6) {
      print(v6);
    }
    let v7 = 0;
    while (v7 < 6) {
        const v9 = v7++;
    }
  if (repeat) { v0--; repeat = false; }
}
}
main();

I'm not sure if this assertion failure has security implications. It looks like it could lead to an OOB access into a strings table in this code (from the same function):

// Perform a "unit" lookup when |unsigned(input) < unsigned(base)|.
Label lengthTwo, done;
branch32(Assembler::AboveOrEqual, input, Imm32(base), &lengthTwo);
{
  move32(input, scratch1);
  toChar(scratch1);

  loadStringFromUnit(scratch1, dest, staticStrings);

  jump(&done);
}

but I haven't verified that. Further, I'm not sure if the root cause of the bug can lead to other symptoms as well, so I'm filing this as a security issue as a precaution.

Group: core-security → javascript-core-security

NI anba because I think this is from recent changes.

Flags: needinfo?(andrebargull)

It looks like it could lead to an OOB access into a strings table in this code (from the same function): [...]

The base input is guaranteed to be MGuardInt32Range, so at runtime we won't execute the generated code when base < 2 or base > 36. So this is only a division by zero bug in release builds.

Flags: needinfo?(andrebargull)
Assignee: nobody → andrebargull
Status: NEW → ASSIGNED
Blocks: sm-security
Severity: -- → N/A
Priority: -- → P1

So this is only a division by zero bug in release builds.

Does that mean we can unhide this bug?

Flags: needinfo?(andrebargull)

I think it's okay to unhide, because it's only a division-by-zero crash in release builds.

Flags: needinfo?(andrebargull)
Group: javascript-core-security
Pushed by andre.bargull@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/406bd52a4879
Check constant base value when lowering Int32ToStringWithBase. r=jandem
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 110 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: