Open Bug 2078790 Opened 1 month ago Updated 1 month ago

Wrong binding for Infinity/NaN in nested with: stale PropertyResult ignoreProtoChain_ flag carried across environments in js::LookupName

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

UNCONFIRMED

People

(Reporter: bugmon, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: testcase)

Attachments

(3 files)

PropertyResult::setNotFound() resets only kind_ and leaves ignoreProtoChain_ untouched, while js::LookupName passes one PropertyResult to LookupProperty for every environment on the chain. When a with environment over a typed array is asked for a canonical-numeric name such as Infinity or NaN, NativeLookupOwnPropertyInline calls setTypedArrayOutOfRange(), and the flag stays set for the next environment. There, NativeLookupPropertyInline sees the own lookup miss, reads shouldIgnoreProtoChain() as true, and stops before visiting that object's prototype, so an inherited property on the enclosing with object is skipped and the name resolves further out (the global Infinity/NaN).

A web page script can make SpiderMonkey resolve an identifier to the wrong binding by reading Infinity or NaN inside a with over a typed array nested in a with over an object that inherits a property of that name. The demonstrated effect is a wrong value returned to the script itself, contrary to the ECMAScript specification and to V8; the same wrong value is produced with --no-jit-backend and with --ion-eager. No crash, assertion failure or memory corruption was observed in the debug shell, and no cross-origin or privilege effect was demonstrated.

Build Info

Affected Code

File: js/src/vm/PropertyResult.h, line 78-104

  void setNotFound() { kind_ = Kind::NotFound; }
  ...
  void setTypedArrayOutOfRange() {
    kind_ = Kind::NotFound;
    ignoreProtoChain_ = IgnoreProtoChain::TypedArrayOutOfRange;
  }
  void setRecursiveResolve() {
    kind_ = Kind::NotFound;
    ignoreProtoChain_ = IgnoreProtoChain::RecursiveResolve;
  }

File: js/src/vm/NativeObject-inl.h, line 753-763

  if (obj->template is<TypedArrayObject>()) {
    if (mozilla::Maybe<uint64_t> index = ToTypedArrayIndex(id)) {
      uint64_t idx = index.value();
      if (idx < obj->template as<TypedArrayObject>().length().valueOr(0)) {
        propp->setTypedArrayElement(idx);
      } else {
        propp->setTypedArrayOutOfRange();
      }
      return true;
    }
  }

File: js/src/vm/NativeObject-inl.h, line 829-847

  while (true) {
    if (!NativeLookupOwnPropertyInline<allowGC, resolveMode>(cx, current, id,
                                                             propp)) {
      return false;
    }

    if (propp->isFound()) {
      objp.set(current);
      return true;
    }

    if (propp->shouldIgnoreProtoChain()) {
      break;
    }

    JSObject* proto = current->staticPrototype();
    if (!proto) {
      break;
    }

File: js/src/vm/JSObject.cpp, line 1152-1171

bool js::LookupName(JSContext* cx, Handle<PropertyName*> name,
                    HandleObject envChain, MutableHandleObject objp,
                    MutableHandleObject pobjp, PropertyResult* propp) {
  RootedId id(cx, NameToId(name));

  for (RootedObject env(cx, envChain); env; env = env->enclosingEnvironment()) {
    if (!LookupProperty(cx, env, id, pobjp, propp)) {
      return false;
    }
    if (propp->isFound()) {
      objp.set(env);
      return true;
    }
  }

  objp.set(nullptr);
  pobjp.set(nullptr);
  propp->setNotFound();
  return true;
}

File: js/src/vm/EnvironmentObject.cpp, line 843-846

  RootedObject actual(cx, &obj->as<WithEnvironmentObject>().object());
  if (!LookupProperty(cx, actual, id, objp, propp)) {
    return false;
  }

The flag is set by the typed array lookup, never cleared by setNotFound(), and consulted on the next environment because LookupName reuses the same PropertyResult. with_LookupProperty forwards the caller's PropertyResult unchanged to LookupProperty on the with-object. Controls run in this verification support this: replacing the inner typed array with a plain object, or giving the outer object an own (not inherited) Infinity, makes SpiderMonkey return the correct value.

Exploit Chain

  1. Script creates an object obj whose prototype has a property named Infinity (value 42), and a typed array ta.
  2. Script evaluates the identifier Infinity inside with (obj) { with (ta) { ... } }; GetEnvironmentName (js/src/vm/Interpreter-inl.h:157-159) calls js::LookupName with one PropertyResult.
  3. For the inner with-environment, with_LookupProperty forwards to LookupProperty on ta; ToTypedArrayIndex treats "Infinity" as a typed array index, it is out of range, and setTypedArrayOutOfRange() sets ignoreProtoChain_.
  4. For the outer with-environment, the own lookup on obj misses and calls setNotFound(), which leaves ignoreProtoChain_ set; NativeLookupPropertyInline breaks out before visiting obj's prototype.
  5. The lookup continues to the global, and the script receives the global Infinity instead of 42. Per the specification (Object Environment Record HasBinding -> HasProperty(obj) -> OrdinaryHasProperty consults the prototype) the result must be 42, which is what V8 returns.

Steps to Reproduce

  1. Use an unmodified tree and build the SpiderMonkey debug shell with build_firefox and /app/mozconfigs/mozconfig.js.linux.debug (output: /firefox/obj-js-debug/dist/bin/js).
  2. Run testcase.js through js_shell_differential_evaluator with shell_a=/firefox/obj-js-debug/dist/bin/js (spidermonkey) and shell_b=/app/tools/d8/d8 (v8), no extra flags: result is differs=true (SpiderMonkey exit 3, Error: got Infinity; d8 exit 0).
  3. Repeat with flags_a=["--differential-testing"]: result is again differs=true with the same exit codes.
  4. No prefs, source patches or additional flags are required.

Security Impact

  • Severity: Low
  • Attacker capability: Script can cause the identifiers Infinity and NaN (both verified) to resolve to the global binding instead of a property inherited by an enclosing with object, when a with over a typed array sits between them. The observed effect is an incorrect value inside the script's own execution. No memory corruption, assertion failure, or cross-boundary effect was demonstrated; the debug shell ran without asserting.
  • Preconditions: Sloppy-mode code using nested with statements, with a typed array as the inner scope object and an outer scope object that has the name only on its prototype chain. No prefs or non-default configuration.

ASAN Report

No sanitizer output: this is a js_differential finding; the proof is the divergence below (verifier's own runs, clean tree at bf9757d91e2e).

=== Run 1 (no extra flags) ===
shell_a (spidermonkey) argv: /firefox/obj-js-debug/dist/bin/js --fuzzing-safe /tmp/fx_audit_jsdiff_qncr10by/testcase.js
shell_a exit_code: 3, timed_out: false, succeeded: false
shell_a stderr:
/tmp/fx_audit_jsdiff_qncr10by/testcase.js:11:9 Error: got Infinity
Stack:
  @/tmp/fx_audit_jsdiff_qncr10by/testcase.js:11:9
shell_b (v8) argv: /app/tools/d8/d8 /tmp/fx_audit_jsdiff_qncr10by/testcase.js
shell_b exit_code: 0, timed_out: false, succeeded: true
differs: true

=== Run 2 (--differential-testing on the SpiderMonkey side) ===
shell_a (spidermonkey) argv: /firefox/obj-js-debug/dist/bin/js --fuzzing-safe --differential-testing /tmp/fx_audit_jsdiff_us2oqsdx/testcase.js
shell_a exit_code: 3, timed_out: false, succeeded: false
shell_a stderr:
/tmp/fx_audit_jsdiff_us2oqsdx/testcase.js:11:9 Error: got Infinity
Stack:
  @/tmp/fx_audit_jsdiff_us2oqsdx/testcase.js:11:1
shell_b (v8) argv: /app/tools/d8/d8 /tmp/fx_audit_jsdiff_us2oqsdx/testcase.js
shell_b exit_code: 0, timed_out: false, succeeded: true
differs: true

=== Additional verifier runs ===
- Controls (inner `with` over a plain object; outer object with an own `Infinity`): SpiderMonkey and d8 both exit 0, differs=false.
- Same testcase, SpiderMonkey `--no-jit-backend` vs `--ion-eager --no-threads`: both exit 3 (differs=false), so the wrong value is not tier-specific.
- Variant using `NaN` inherited from the outer object, read inside a function, typed array Uint8Array(4): SpiderMonkey exit 3, d8 exit 0, differs=true.
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
Blocks: sm-security
Severity: -- → S3
Priority: -- → P3

This looks like it's purely a correctness issue.

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: