Open Bug 2078751 Opened 1 month ago Updated 1 month ago

Writing `length` on a sealed/frozen strict-mode arguments object throws TypeError (ArgumentsObject::reifyLength after SetIntegrityLevel fast path)

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

UNCONFIRMED

People

(Reporter: bugmon, Unassigned)

Details

(Keywords: testcase)

Attachments

(3 files)

js::SetIntegrityLevel sends UnmappedArgumentsObject through the NativeObject::freezeOrSealProperties fast path, which makes the lazily-resolved length custom data property non-configurable without reifying it. Any later non-resolving define of length goes through ArgumentsObject::reifyLength, which redefines the property as writable, non-enumerable and configurable using the throwing overload of NativeDefineDataProperty; that redefinition is rejected with JSMSG_CANT_REDEFINE_PROP, so Reflect.set, plain assignment, Object.defineProperty and Array.prototype.splice.call throw a TypeError where ECMA-262 requires the write to succeed.

A web page script can make SpiderMonkey throw a TypeError the specification does not allow by sealing or freezing a strict-mode arguments object and then writing or redefining its length. The demonstrated effect is incorrect script behaviour only: SpiderMonkey exits with an uncaught TypeError where V8 completes, with and without --differential-testing. No memory corruption, assertion failure or JIT-tier divergence was observed.

Build Info

Affected Code

File: js/src/vm/JSObject.cpp, line 516-528

  if (obj->is<NativeObject>() && !obj->is<TypedArrayObject>() &&
      !obj->is<MappedArgumentsObject>()) {
    Handle<NativeObject*> nobj = obj.as<NativeObject>();

    // Use a fast path to seal/freeze properties. This has the benefit of
    // creating shared property maps if possible, whereas the slower/generic
    // implementation below ends up converting non-empty objects to dictionary
    // mode.
    if (nobj->shape()->propMapLength() > 0) {
      if (!NativeObject::freezeOrSealProperties(cx, nobj, level)) {
        return false;
      }
    }

File: js/src/vm/NativeObject.cpp, line 1670-1679

  } else if (obj->is<ArgumentsObject>() && !desc_.resolving()) {
    auto argsobj = HandleObject(obj).as<ArgumentsObject>();
    if (id.isAtom(cx->names().length)) {
      // Either we are resolving the .length property on this object,
      // or redefining it. In the latter case only, we must reify the
      // property.
      if (!ArgumentsObject::reifyLength(cx, argsobj)) {
        return false;
      }

File: js/src/vm/ArgumentsObject.cpp, line 600-613

bool ArgumentsObject::reifyLength(JSContext* cx, Handle<ArgumentsObject*> obj) {
  if (obj->hasOverriddenLength()) {
    return true;
  }

  RootedId id(cx, NameToId(cx->names().length));
  RootedValue val(cx, Int32Value(obj->initialLength()));
  if (!NativeDefineDataProperty(cx, obj, id, val, JSPROP_RESOLVING)) {
    return false;
  }

  obj->markLengthOverridden();
  return true;
}

File: js/src/vm/NativeObject.cpp, line 1924-1939

bool js::NativeDefineDataProperty(JSContext* cx, Handle<NativeObject*> obj,
                                  HandleId id, HandleValue value,
                                  unsigned attrs) {
  ObjectOpResult result;
  if (!NativeDefineDataProperty(cx, obj, id, value, attrs, result)) {
    return false;
  }
  if (!result) {
    // Off-thread callers should not get here: they must call this
    // function only with known-valid arguments. Populating a new
    // PlainObject with configurable properties is fine.
    result.reportError(cx, obj, id);
    return false;
  }
  return true;

Sealing takes the shape-level fast path for every native object except typed arrays and mapped arguments objects, so an unmapped arguments object keeps length as an unreified custom data property that is now non-configurable. A later define of length reifies first, and the reification asks for a configurable property through the overload that reports failure as a thrown error. Controls I ran succeed in SpiderMonkey: a sealed mapped (sloppy) arguments object, an unmapped one made non-configurable with Object.defineProperty(args, "length", {configurable: false}), and an unmapped one whose length was assigned before sealing.

Exploit Chain

  1. Script calls a strict-mode function and obtains its arguments object, an UnmappedArgumentsObject whose length is a lazily-resolved custom data property.
  2. Script calls Object.seal(args) (or Object.freeze(args)); js::SetIntegrityLevel uses NativeObject::freezeOrSealProperties, clearing the configurable flag on length while the length-overridden bit stays unset.
  3. Script calls Reflect.set(args, "length", 5); UnmappedArgSetter calls NativeDefineProperty, which calls ArgumentsObject::reifyLength.
  4. reifyLength tries to redefine length as configurable via the throwing NativeDefineDataProperty overload; the define fails with JSMSG_CANT_REDEFINE_PROP and a TypeError propagates out of Reflect.set, which per ECMA-262 must return true here.
  5. Demonstrated impact ends there: a spurious TypeError. No memory-safety consequence was observed.

Steps to Reproduce

  1. Build the SpiderMonkey debug shell from an unmodified tree with /app/mozconfigs/mozconfig.js.linux.debug (build dir /firefox/obj-js-debug). No prefs or source patches.
  2. Run testcase.js with js_shell_differential_evaluator: shell_a=/firefox/obj-js-debug/dist/bin/js (spidermonkey), shell_b=/app/tools/d8/d8 (v8), no extra flags. Result: differs=true; SpiderMonkey exit 3, d8 exit 0.
  3. Run again with flags_a=["--differential-testing"]. Result: differs=true; SpiderMonkey exit 3, d8 exit 0.
  4. Testcase:
    function makeUnmapped() {
    "use strict";
    return arguments;
    }
    var args = makeUnmapped(1, 2);
    Object.seal(args);
    var desc = Object.getOwnPropertyDescriptor(args, "length");
    if (desc.value !== 2 || desc.writable !== true || desc.configurable !== false) {
    throw new Error("unexpected descriptor after seal");
    }
    var ok = Reflect.set(args, "length", 5);
    if (ok !== true) {
    throw new Error("Reflect.set returned " + ok);
    }
    if (args.length !== 5) {
    throw new Error("length is " + args.length);
    }

Security Impact

  • Severity: Low
  • Attacker capability: Script can trigger a TypeError that the specification does not permit when writing or redefining length on a sealed or frozen strict-mode arguments object. This is a conformance defect; no memory corruption, assertion failure, information disclosure or JIT-tier divergence was demonstrated.
  • Preconditions: None beyond running JavaScript; the path uses only standard language features (strict-mode arguments, Object.seal/Object.freeze, Reflect.set).

ASAN Report

No sanitizer report: this is a js_differential finding. Verifier's independent runs (js_shell_differential_evaluator, debug shell built from clean tree):

=== Run 1 (no extra flags) ===
differs: true
shell_a (spidermonkey) argv: /firefox/obj-js-debug/dist/bin/js --fuzzing-safe /tmp/fx_audit_jsdiff_sao1hhhf/testcase.js
shell_a exit_code: 3, timed_out: false, succeeded: false
shell_b (v8) argv: /app/tools/d8/d8 /tmp/fx_audit_jsdiff_sao1hhhf/testcase.js
shell_b exit_code: 0, timed_out: false, succeeded: true
shell_a stderr:
/tmp/fx_audit_jsdiff_sao1hhhf/testcase.js:11:18 TypeError: can't redefine non-configurable property "length"
Stack:
  @/tmp/fx_audit_jsdiff_sao1hhhf/testcase.js:11:18
shell_b stderr: empty

=== Run 2 (--differential-testing on the SpiderMonkey side) ===
differs: true
shell_a (spidermonkey) argv: /firefox/obj-js-debug/dist/bin/js --fuzzing-safe --differential-testing /tmp/fx_audit_jsdiff_ilkaaqtv/testcase.js
shell_a exit_code: 3, timed_out: false, succeeded: false
shell_b (v8) argv: /app/tools/d8/d8 /tmp/fx_audit_jsdiff_ilkaaqtv/testcase.js
shell_b exit_code: 0, timed_out: false, succeeded: true
shell_a stderr:
/tmp/fx_audit_jsdiff_ilkaaqtv/testcase.js:11:18 TypeError: can't redefine non-configurable property "length"
Stack:
  @/tmp/fx_audit_jsdiff_ilkaaqtv/testcase.js:11:1
shell_b stderr: empty

=== Variant run (same pairing, no extra flags; per-variant try/catch output) ===
SpiderMonkey:
sloppy assign: threw TypeError
defineProperty value: threw TypeError
splice: threw TypeError
frozen Reflect.defineProperty same value: threw TypeError
sealed Reflect.set: threw TypeError
sealed iterator set: ok true
sealed element set: ok true 9
sealed element define: ok true 9
d8:
sloppy assign: ok 5
defineProperty value: ok 7
splice: ok 2
frozen Reflect.defineProperty same value: ok true
sealed Reflect.set: ok true
sealed iterator set: ok true
sealed element set: ok true 9
sealed element define: ok true 9

=== Control run (sealed mapped arguments; unmapped made non-configurable via defineProperty; sealed plain object; unmapped with length assigned before seal) ===
differs: false; both shells exit 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
Blocks: sm-security
Severity: -- → S3
Priority: -- → P3
No longer blocks: sm-security
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: