Open Bug 2078703 Opened 1 month ago Updated 1 month ago

Megamorphic SetProp IC stub drops strict-mode ReferenceError for assignment to undeclared global [@ js::jit::SetPropertyMegamorphic]

Categories

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

defect

Tracking

()

UNCONFIRMED

People

(Reporter: bugmon, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: testcase)

Attachments

(4 files)

js::jit::SetPropertyMegamorphic (js/src/jit/VMFunctions.cpp) takes no pc or op and always performs a qualified set, yet SetPropIRGenerator::tryAttachMegamorphicSetSlot attaches the MegamorphicStoreSlot stub for every JOF_PROPSET op, which includes SetName, StrictSetName, SetGName and StrictSetGName. The interpreter's SetNameOperation and the non-megamorphic jit::SetProperty use NativeSetProperty<Unqualified> for those ops, which is the only path that reports JSMSG_UNDECLARED_VAR. Once a strict-mode name-assignment site goes megamorphic, an assignment to an undeclared global defines the global property instead of throwing ReferenceError.

A script author can make a strict-mode assignment to an undeclared global silently create a global property instead of throwing ReferenceError by driving that assignment site's inline cache into megamorphic mode. This was demonstrated in the SpiderMonkey shell as a wrong-answer divergence between the C++ interpreter and the Baseline IC tiers (also with default tiering and --ion-eager); V8 matches the interpreter. No assertion failure or memory-safety effect was observed in the debug build.

Build Info

Affected Code

File: js/src/jit/VMFunctions.cpp, line 2451-2469

template <bool Cached>
bool SetPropertyMegamorphic(JSContext* cx, HandleObject obj, HandleId id,
                            HandleValue value, bool strict) {
  if (obj->is<NativeObject>() &&
      obj.as<NativeObject>()->canDoSetPropertyFastpath()) {
    bool optimized = false;
    if (!TryAddOrSetNativeObjectProperty<Cached>(cx, obj.as<NativeObject>(), id,
                                                 value, &optimized)) {
      return false;
    }
    if (optimized) {
      return true;
    }
  }
  Rooted<Value> receiver(cx, ObjectValue(*obj));
  ObjectOpResult result;
  return SetProperty(cx, obj, id, value, receiver, result) &&
         result.checkStrictModeError(cx, obj, id, strict);
}

File: js/src/jit/CacheIR.cpp, line 4386-4394

    if (nameOrSymbol) {
      TRY_ATTACH(tryAttachNativeSetSlot(obj, objId, id, rhsValId));
      if (IsPropertySetOp(JSOp(*pc_))) {
        TRY_ATTACH(tryAttachSetArrayLength(obj, objId, id, rhsValId));
        TRY_ATTACH(tryAttachSetter(obj, objId, id, rhsValId));
        TRY_ATTACH(tryAttachWindowProxy(obj, objId, id, rhsValId));
        TRY_ATTACH(tryAttachProxy(obj, objId, id, rhsValId));
        TRY_ATTACH(tryAttachMegamorphicSetSlot(obj, objId, id, rhsValId));
      }

File: js/src/jit/CacheIR.cpp, line 5361-5370

AttachDecision SetPropIRGenerator::tryAttachMegamorphicSetSlot(
    HandleObject obj, ObjOperandId objId, HandleId id, ValOperandId rhsId) {
  if (mode_ != ICState::Mode::Megamorphic || cacheKind_ != CacheKind::SetProp) {
    return AttachDecision::NoAction;
  }

  writer.megamorphicStoreSlot(objId, id, rhsId, IsStrictSetPC(pc_));
  trackAttached("SetProp.MegamorphicNativeSlot");
  return AttachDecision::Attach;
}

File: js/src/jit/CacheIRCompiler.cpp, line 9873-9875

  using Fn = bool (*)(JSContext*, HandleObject, HandleId, HandleValue, bool);
  callvm.callNoResult<Fn, SetPropertyMegamorphic<false>>();
  return true;

File: js/src/jit/VMFunctions.cpp, line 896-922

bool SetProperty(JSContext* cx, HandleObject obj, Handle<PropertyName*> name,
                 HandleValue value, bool strict, jsbytecode* pc) {
  RootedId id(cx, NameToId(name));

  RootedValue receiver(cx, ObjectValue(*obj));
  ObjectOpResult result;
  if (MOZ_LIKELY(!obj->getOpsSetProperty())) {
    JSOp op = JSOp(*pc);
    if (op == JSOp::SetName || op == JSOp::StrictSetName ||
        op == JSOp::SetGName || op == JSOp::StrictSetGName) {
      if (!NativeSetProperty<Unqualified>(cx, obj.as<NativeObject>(), id, value,
                                          receiver, result)) {
        return false;
      }
    } else {
      if (!NativeSetProperty<Qualified>(cx, obj.as<NativeObject>(), id, value,
                                        receiver, result)) {
        return false;
      }
    }
  } else {
    if (!SetProperty(cx, obj, id, value, receiver, result)) {
      return false;
    }
  }
  return result.checkStrictModeError(cx, obj, id, strict);
}

File: js/src/vm/NativeObject.cpp, line 2579-2584

  if (!IsQualified && receiver.isObject() &&
      receiver.toObject().isUnqualifiedVarObj()) {
    if (!MaybeReportUndeclaredVarAssignment(cx, id)) {
      return false;
    }
  }

The megamorphic VM helper falls through to a qualified SetProperty (or to the add/set fast path) without distinguishing name ops from property ops. The stub that calls it is attached under IsPropertySetOp, which is true for the four SetName/SetGName opcodes because they carry JOF_PROPSET (js/src/vm/Opcodes.h:3040, 3055, 3065, 3075). The non-megamorphic helper in the same file and the interpreter both select NativeSetProperty<Unqualified> for those ops; SetNonexistentProperty only calls MaybeReportUndeclaredVarAssignment when IsQualified is false (js/src/vm/NativeObject.cpp:2579-2583). In the shell, the global class has a resolve hook (js/src/shell/js.cpp:11443), so canDoSetPropertyFastpath() is false (js/src/vm/NativeObject-inl.h:597) and, by source reading, the qualified SetProperty fallback is the branch taken.

Exploit Chain

  1. A strict-mode function f contains an unqualified assignment gx = i, compiled to BindUnqualifiedGName + StrictSetGName.
  2. globalThis.gx is defined, then f is called 100 times while a new global property is added on each iteration, so the global's shape changes each call and the StrictSetGName IC accumulates shape-guarded stubs until it transitions to megamorphic mode and attaches MegamorphicStoreSlot.
  3. delete globalThis.gx removes the binding, making the next gx = 1 an assignment to an unresolvable reference.
  4. f(1) runs the megamorphic stub, which calls SetPropertyMegamorphic<false>; that performs a qualified set on the global object and never reaches MaybeReportUndeclaredVarAssignment.
  5. The assignment succeeds and defines globalThis.gx (typeof number) instead of throwing ReferenceError; the testcase detects this and throws its own Error, giving exit 3 on the IC-enabled side while the C++ interpreter exits 0.

Steps to Reproduce

  1. Build the SpiderMonkey debug shell from the unmodified tree with /app/mozconfigs/mozconfig.js.linux.debug (no source patches, no prefs).
  2. Run testcase.js with js_shell_differential_evaluator using /firefox/obj-js-debug/dist/bin/js for both shells: flags_a = ["--no-threads", "--no-jit-backend"], flags_b = ["--no-threads", "--baseline-eager"]. Result: differs true (shell_a exit 0, shell_b exit 3).
  3. Re-run with "--differential-testing" appended to both flag sets. Result: differs true (exit 0 vs exit 3).
  4. Bisection: flags_a = ["--no-threads", "--baseline-eager", "--cache-ir-stubs=off", "--differential-testing"] exits 0; flags_b = ["--no-threads", "--baseline-eager", "--differential-testing"] exits 3.
  5. Cross-check: the same testcase exits 0 in /app/tools/d8/d8, and exits 3 in the SpiderMonkey shell with only ["--no-threads", "--differential-testing"] (default tiering) and with ["--no-threads", "--ion-eager"].

Security Impact

  • Severity: Low
  • Attacker capability: Script can cause a strict-mode assignment to an undeclared global variable to define a global property instead of throwing ReferenceError, once the assignment site's inline cache is megamorphic. This is a language-semantics correctness defect; no assertion failure, crash, or memory-safety effect was observed in the debug shell.
  • Preconditions: The strict-mode name-assignment site must have seen enough distinct global shapes to enter megamorphic IC mode with CacheIR stubs enabled (the default). Demonstrated in the JS shell only; no flags beyond tier/determinism pins are required.

ASAN Report

No sanitizer report: this is a wrong-answer divergence (js_differential), verified independently on a clean tree.

Run 1 (differs: true)
shell_a argv: /firefox/obj-js-debug/dist/bin/js --fuzzing-safe --no-threads --no-jit-backend testcase.js -> exit 0
shell_b argv: /firefox/obj-js-debug/dist/bin/js --fuzzing-safe --no-threads --baseline-eager testcase.js -> exit 3
shell_b stderr:
testcase.js:18:9 Error: strict-mode assignment to undeclared global did not throw ReferenceError; typeof gx = number
Stack:
  @testcase.js:18:9

Run 2, --differential-testing gate (differs: true)
shell_a argv: /firefox/obj-js-debug/dist/bin/js --fuzzing-safe --no-threads --no-jit-backend --differential-testing testcase.js -> exit 0
shell_b argv: /firefox/obj-js-debug/dist/bin/js --fuzzing-safe --no-threads --baseline-eager --differential-testing testcase.js -> exit 3
shell_b stderr:
testcase.js:18:9 Error: strict-mode assignment to undeclared global did not throw ReferenceError; typeof gx = number
Stack:
  @testcase.js:18:1

Run 3, bisection (differs: true)
shell_a argv: js --fuzzing-safe --no-threads --baseline-eager --cache-ir-stubs=off --differential-testing testcase.js -> exit 0
shell_b argv: js --fuzzing-safe --no-threads --baseline-eager --differential-testing testcase.js -> exit 3

Run 4, V8 cross-check (differs: true)
shell_a argv: js --fuzzing-safe --no-threads --differential-testing testcase.js -> exit 3
shell_b argv: /app/tools/d8/d8 testcase.js -> exit 0

Run 5, js_shell_evaluator with [--no-threads, --ion-eager]: crashed false, exit 3 (same thrown Error; no assertion).
Attached patch fix.patch — — Splinter Review
Attached file crash_stack.txt —
Attached file testcase.js —
Attached file README.txt —

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
Blocks: sm-security
Severity: -- → S3
Priority: -- → P3

This is just a correctness issue where we don't throw an error. It is not exploitable and extremely unlikely to be a webcompat problem.

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

Attachment

General

Created:
Updated:
Size: