2042998 - Assertion failure: idval.isSymbol(), at js/src/vm/Interpreter-inl.h:524 (with Debugger.Frame.eval)

Status: NEW

(Core :: JavaScript: Debugger API, defect, P3)

Description

[Christian Holler (:decoder)]

Summary

The per-name private-field cache built for Debugger.Frame.prototype.eval does not respect lexical shadowing: when two enclosing classes declare a private name with the same spelling, the outer class's entry overwrites the inner one. This routes private-name accesses through the wrong path (field-access vs. method brand-check), producing a debug-build assertion in js::CheckPrivateFieldOperation and a wrong-brand load in js::LoadAliasedDebugVar. Both defects are reachable from script via Debugger.Frame.eval.

Root cause

js::ScopeContext::cachePrivateFieldsForEval at js/src/frontend/Stencil.cpp:1065 walks scopes inner -> outer and stores each private name in a HashMap using put(name, NameLocation) — last-write-wins. The outermost declaration of any shadowed private name therefore wins:

for (InputScopeIter si(effectiveScope); si; si++) {       // inner -> outer
  if (si.scope().kind() == ScopeKind::ClassBody) {
    for (auto bi = InputBindingIter(scope_ref); bi; bi++) {
      if (bi.kind() == BindingKind::PrivateMethod || ...) {
        NameLocation loc = NameLocation::DebugEnvironmentCoordinate(bi.kind(), hops, slots);
        effectiveScopePrivateFieldCache_->put(parserName, loc);  // overwrites inner
      }
      slots++;
    }
  }
  hops++;
}

The consumer, js::frontend::EmitterScope::lookupPrivate at js/src/frontend/EmitterScope.cpp:1087, trusts that single entry's bindingKind() to decide whether to emit a brand check (GetAliasedDebugVar) or a field access (GetName + CheckPrivateField).

This yields two defects, both confined to the eval-in-frame path (getPrivateFieldLocation asserts enclosingEnvironmentIsDebugProxy_, so GetAliasedDebugVar/js::LoadAliasedDebugVar is reachable exclusively from Debugger.Frame.prototype.eval):

1. Type-invariant violation / assertion (primary). Inner private method shadowed by outer non-method (field, getter/setter, static method, static field). Private methods have their private-name binding optimized away (BytecodeEmitter::emitNewPrivateNames at js/src/frontend/BytecodeEmitter.cpp:12070 — isOptimized = true, no emitNewPrivateName); only the synthetic .privateBrand exists. The mis-classified field path emits GetName "#x" + CheckPrivateField, but the loaded value is not a private-name Symbol, tripping MOZ_ASSERT(idval.isSymbol()) in js::CheckPrivateFieldOperation (js/src/vm/Interpreter-inl.h:524).

2. Wrong brand returned by js::LoadAliasedDebugVar (secondary). When both declarations are private methods, the brand-check path runs but LoadAliasedDebugVar (js/src/vm/Interpreter.cpp:5383) walks the wrong number of hops on the environment chain and returns the outer class's brand. An instance of the outer class then wrongly passes the inner class's brand check, and the inner class's private method is invoked on it — a private-name integrity violation.

A sweep over (outer kind) x (inner kind) x (access form) shows the assertion in (1) fires for every combination where the inner declaration is an instance method and the outer same-named declaration is a non-method, for accesses o.#x, o.#x(), #x in o, o.#x = 5 (20 combinations).

In an optimized build the MOZ_ASSERT in (1) is compiled out, the non-Symbol value flows into HasOwnProperty which converts via ToPropertyKey and reports a benign TypeError. Defect (2) is silent in both builds and is a private-method-dispatch soundness hole.

Reproduction

Debug ASan shell build. Testcases are attached: poc.js (primary, asserts) and poc_wrong_brand.js (secondary, wrong brand).

$ m-c-asan-debug/dist/bin/js --fuzzing-safe poc.js

poc.js (excerpt):

var dbgGlobal = newGlobal({ newCompartment: true });
var dbg = new dbgGlobal.Debugger();
function evalInNewestFrame(code) {
  dbg.addDebuggee(this);
  var frame = dbg.getNewestFrame().older;
  return frame.eval(code);
}
class A {
  #x = 99;                           // outer: private FIELD
  trigger() {
    class B {
      #x() { return "B-method"; }    // inner: private METHOD (shadows)
      run() { return evalInNewestFrame("this.#x"); }
    }
    return new B().run();
  }
}
new A().trigger();

No special prefs required; default debug ASan build is sufficient. The shell intrinsics used (newGlobal, Debugger) correspond to the chrome-only Debugger API in the browser.

Crash signature (debug ASan)

Assertion failure: idval.isSymbol(), at js/src/vm/Interpreter-inl.h:524
#1  js::CheckPrivateFieldOperation(...)            Interpreter-inl.h:524
#2  js::Interpret(...)                             Interpreter.cpp:2348      (JSOp::CheckPrivateField)
#5  js::ExecuteKernel(...)                         Interpreter.cpp:858
#6  EvaluateInEnv(...)                             debugger/Frame.cpp:1158
#7  js::DebuggerGenericEval(...)                   debugger/Frame.cpp:1249
#8  js::DebuggerFrame::eval(...)                   debugger/Frame.cpp:1270
AddressSanitizer: SEGV on unknown address 0x000000000000 (WRITE)  [from MOZ_CrashSequence]

Full log: crash_debug_asan.txt.

Suggested fix

cachePrivateFieldsForEval should respect lexical shadowing. Since the walk is inner -> outer, the simplest fix is: only insert into the cache when no entry for that name exists yet (i.e. first/innermost declaration wins) — e.g. replace the unconditional put with lookupForAdd+add, or check !cache.has(parserName) before inserting. lookupPrivate then naturally relies on the binding kind of the innermost declaration. Equivalently, key the cache by (name, scope-depth) so shadowed declarations of different kinds are not conflated.

Security impact

• Crash is a debug-build assertion (broken type invariant). Release builds convert the type confusion to a benign TypeError via ToPropertyKey in HasOwnProperty. No memory corruption observed on this path.
• Trigger requires Debugger.Frame.prototype.eval — chrome-only / shell-only API, not reachable from web content.
• Secondary defect (wrong brand from LoadAliasedDebugVar) is a private-name integrity violation but again only reachable via Debugger API.

Marked sec-audit pending Mozilla's view on Debugger-API-only correctness/integrity bugs.

Comment 1

[Christian Holler (:decoder)]

Attachment: poc.js — primary testcase (assertion failure)

Comment 2

[Christian Holler (:decoder)]

Attachment: poc_wrong_brand.js — secondary testcase (wrong brand dispatch)