Open Bug 2076701 Opened 1 month ago Updated 1 month ago

Ion miscompiles a BigInt comparison at the signed 64-bit limit

Categories

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

defect

Tracking

()

UNCONFIRMED

People

(Reporter: decoder, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: testcase)

Attachments

(3 files)

MCompare::tryFoldBigIntPtr in js/src/jit/MIR.cpp:6024 folds a comparison against a BigInt constant outside intptr_t range without preserving the speculative producer’s overflow bailout. GVN can discard the unused MBigIntPtrAdd through js/src/jit/ValueNumbering.cpp:859, removing the overflow check emitted at js/src/jit/CodeGenerator.cpp:12313. Consequently, the submitted comparison of 9223372036854775807n + 1n against 9223372036854775808n returns true instead of false.

A web-content author can cause incorrect JavaScript control flow by supplying boundary-valued BigInts to an Ion-optimized comparison. Independent debug-shell runs reproduce the wrong result with and without --differential-testing, while disabling GVN restores correctness. Memory corruption was not demonstrated.

This finding was discovered during variant analysis seeded by bug 1736307.

Build Info

Affected Code

File: js/src/jit/MIR.cpp, line 6024-6036

// Extract the BigInt value if representable as intptr_t.
intptr_t value;
if (!BigInt::isIntPtr(bigInt, &value)) {
  // The comparison is a constant if the BigInt has too many digits.
  int32_t repr = bigInt->isNegative() ? -1 : 1;

  bool result;
  if (left == intPtrToBigInt) {
    result = FoldComparison(jsop_, 0, repr);
  } else {
    result = FoldComparison(jsop_, repr, 0);
  }
  return MConstant::NewBoolean(alloc, result);

File: js/src/jit/ValueNumbering.cpp, line 859-865

// Discard |def| if it's now unused. Similar to guards, we allow to replace
// effectful instructions when the node's foldsTo method said |def| can be
// replaced.
if (DeadIfUnusedAllowEffectful(def)) {
  if (!discardDefsRecursively(def, AllowEffectful::Yes)) {
    return false;
  }

File: js/src/jit/CodeGenerator.cpp, line 12313-12315

Label bail;
masm.branchAddPtr(Assembler::Overflow, lhs, output, &bail);
bailoutFrom(&bail, ins->snapshot());

The constant comparison no longer depends on the checked arithmetic result. The producer is movable and has no aliases in js/src/jit/MIR.h:5652, so the unused operation and its overflow bailout can be removed.

Exploit Chain

  1. Execute a function that returns value + 1n < 9223372036854775808n.
  2. Warm the function with small BigInts so Ion specializes its arithmetic to machine-word BigInt operations.
  3. Allow GVN to replace the comparison with a Boolean constant and discard the unused checked addition.
  4. Call the function with 9223372036854775807n and observe true instead of false, selecting an incorrect JavaScript branch.

Steps to Reproduce

  1. Use clean Firefox revision 8445f37d6a14937595eabb96376b4433b4eb67e7 and build it with build_firefox using /app/mozconfigs/mozconfig.js.linux.debug.
  2. Load the contents of /tmp/tmpjw89ufw8/testcase.js into js_shell_differential_evaluator. Set both shells to /firefox/obj-js-debug/dist/bin/js.
  3. Set flags_a to ["--no-jit-backend"] and flags_b to ["--baseline-eager", "--ion-warmup-threshold=30", "--ion-offthread-compile=off"]. Observe differs=true and exit codes 0 and 3.
  4. Repeat with --differential-testing added to both configurations. Observe the same discrepancy and the error "BigInt comparison lost the overflow bailout".
  5. Add --ion-gvn=off to the optimizing configuration. It now exits successfully. No source patches, preferences, or external services are needed.

Security Impact

  • Severity: Low
  • Attacker capability: Cause an Ion-optimized BigInt comparison to produce the wrong Boolean and alter JavaScript control flow. Memory corruption or a privilege-boundary violation was not demonstrated.
  • Preconditions: The affected 64-bit SpiderMonkey build must Ion-compile the comparison after small-BigInt warmup. The testcase uses standard JavaScript and requires no source modification.

ASAN Report

No sanitizer crash was reported; this finding is a verified JS differential.

Without --differential-testing:
A: /firefox/obj-js-debug/dist/bin/js --fuzzing-safe --no-jit-backend /tmp/fx_audit_jsdiff_fzo3zy7a/testcase.js
Exit code: 0
B: /firefox/obj-js-debug/dist/bin/js --fuzzing-safe --baseline-eager --ion-warmup-threshold=30 --ion-offthread-compile=off /tmp/fx_audit_jsdiff_fzo3zy7a/testcase.js
Exit code: 3
differs: true
Stderr:
/tmp/fx_audit_jsdiff_fzo3zy7a/testcase.js:10:9 Error: BigInt comparison lost the overflow bailout
Stack:
  @/tmp/fx_audit_jsdiff_fzo3zy7a/testcase.js:10:9

With --differential-testing:
A: /firefox/obj-js-debug/dist/bin/js --fuzzing-safe --no-jit-backend --differential-testing /tmp/fx_audit_jsdiff_964vf0kr/testcase.js
Exit code: 0
B: /firefox/obj-js-debug/dist/bin/js --fuzzing-safe --baseline-eager --ion-warmup-threshold=30 --ion-offthread-compile=off --differential-testing /tmp/fx_audit_jsdiff_964vf0kr/testcase.js
Exit code: 3
differs: true
Stderr:
/tmp/fx_audit_jsdiff_964vf0kr/testcase.js:10:9 Error: BigInt comparison lost the overflow bailout
Stack:
  @/tmp/fx_audit_jsdiff_964vf0kr/testcase.js:10:1

GVN control: Adding --ion-gvn=off to the gated optimizing configuration changes its exit code from 3 to 0.
Attached patch fix.patch — — Splinter Review
Attached file crash_stack.txt —
Attached file testcase.js —

This bug was filed automatically by an AI-powered security testing tool. The analysis, testcase, and any suggested patches were generated by an AI model and have NOT been verified by a human.

Please review all details carefully before triaging. The issue was confirmed by an independent verifier, but for non-ASAN issues, the verifier has been made less strict. Additionally, the root cause analysis and suggested fix may contain errors.

Group: core-security → javascript-core-security
See Also: → 1736307
Blocks: sm-security
Severity: -- → S3
Priority: -- → P3

This isn't security sensitive.

Group: javascript-core-security
Keywords: sec-moderate
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: