Closed Bug 2042717 Opened 5 months ago Closed 4 months ago

Misaligned GCPtr<Shape*> trailing array in NativeIterator [@ js::NativeIterator::NativeIterator]

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

RESOLVED DUPLICATE of bug 2013993

People

(Reporter: bugmon, Unassigned)

References

(Blocks 2 open bugs)

Details

(5 keywords)

Attachments

(5 files)

SpiderMonkey allocates each NativeIterator with a trailing byte region that holds three variable-length arrays laid out back-to-back: propertyCount IteratorProperty entries (8 bytes each), then — only when the iterator-indices optimization is active (Flags::IndicesAllocated) — propertyCount PropertyIndex entries (4 bytes each), and finally protoShapeCount GCPtr<Shape*> entries (8 bytes each, requiring 8-byte alignment). NativeIterator::protoShapesBegin() computes the start of the GCPtr<Shape*> array by summing those sizes from the 8-byte-aligned base. Because PropertyIndex is only 4 bytes, when indices are allocated AND propertyCount is odd, the PropertyIndex block contributes an odd4 == 4 (mod 8) byte offset, so the GCPtr<Shape> array begins at a 4-byte-aligned (misaligned) address. A GCPtr<Shape*> is then constructed there at Iteration.cpp:922 (new (protoShapesEnd()) GCPtr<Shape*>(shape)) and later loaded and GC-traced — undefined behavior (misaligned construction/access of an 8-byte-aligned GC pointer).

The bug additionally requires the iterator to carry proto-chain shapes (protoShapeCount > 0, i.e. a cacheable prototype chain of length > 1) so that a proto-shape pointer is actually written. The static_asserts in NumTrailingBytes only validate pairwise type alignments and therefore miss this cumulative-offset misalignment. It is reachable from ordinary web JavaScript: for (key in obj) { ... obj[key] ... } over megamorphic objects engages Ion's OptimizeIteratorIndices pass, which calls js::GetIteratorWithIndices -> GetIteratorImpl -> CreatePropertyIterator -> the NativeIterator constructor; using objects with 3 own enumerable data properties (odd count, IndicesAllocated) and the default Object.prototype proto (cacheable chain length 2 => protoShapeCount 1) produces the misaligned GCPtr<Shape*>.

The defect is in the shipping engine source but is undisclosed because Firefox's default UBSan configuration (MOZ_UBSAN) enables only bool,bounds,enum,function,integer-divide-by-zero,pointer-overflow,return,vla-bound,object-size and deliberately omits the alignment check, so Mozilla CI/fuzzers never observe it. It was demonstrated by enabling -fsanitize=alignment for only Iteration.cpp.

Build Info

Affected Code

File: js/src/vm/Iteration.h, line 347-357

GCPtr<Shape*>* protoShapesBegin(size_t numProperties) const {
    uintptr_t raw = reinterpret_cast<uintptr_t>(this);
    uintptr_t propertiesStart = raw + offsetOfFirstProperty();
    uintptr_t propertiesEnd =
        propertiesStart + numProperties * sizeof(IteratorProperty);
    uintptr_t result = propertiesEnd;
    if (flags_ & Flags::IndicesAllocated) {
      result += numProperties * sizeof(PropertyIndex);   // odd*4 == 4 (mod 8) -> misaligns the GCPtr<Shape*> array below
    }
    return reinterpret_cast<GCPtr<Shape*>*>(result);
  }

File: js/src/vm/Iteration.cpp, line 781-792

static inline size_t NumTrailingBytes(size_t propertyCount,
                                      size_t protoShapeCount, bool hasIndices) {
  static_assert(alignof(IteratorProperty) <= alignof(NativeIterator));
  static_assert(alignof(GCPtr<Shape*>) <= alignof(IteratorProperty));
  static_assert(alignof(PropertyIndex) <= alignof(GCPtr<Shape*>));
  size_t result = propertyCount * sizeof(IteratorProperty) +
                  protoShapeCount * sizeof(GCPtr<Shape*>);
  if (hasIndices) {
    result += propertyCount * sizeof(PropertyIndex);   // 4-byte * odd count is not rounded up to 8
  }
  return result;
}

File: js/src/vm/Iteration.cpp, line 918-927

    for (uint32_t i = 0; i < numShapes; i++) {
      MOZ_ASSERT(pobj->is<NativeObject>());
      Shape* shape = pobj->shape();
      if (i > 0) {
        new (protoShapesEnd()) GCPtr<Shape*>(shape);  // <-- line 922: constructs GCPtr<Shape*> at a 4-byte-misaligned address (UB)
        protoShapeCount_++;
      }
      shapesHash = mozilla::AddToHash(shapesHash, HashIteratorShape(shape));
      pobj = pobj->staticPrototype();
    }

File: js/src/vm/Iteration.h, line 88-132

struct PropertyIndex {
 private:
  uint32_t asBits_;        // sizeof == 4, alignof == 4
 ...
};
// IteratorProperty: one uintptr_t raw_  => sizeof == 8, alignof == 8
// GCPtr<Shape*>:    one Shape*          => sizeof == 8, alignof == 8 (the misaligned type)

The trailing-array layout places a 4-byte-element PropertyIndex array between the IteratorProperty array and the 8-byte-aligned GCPtr<Shape*> array. An odd propertyCount makes the PropertyIndex block end at a 4-byte (not 8-byte) boundary, misaligning every subsequent GCPtr<Shape*>. The static_asserts only check pairwise alignof relationships, not the cumulative offset.

Exploit Chain

  1. A page runs for (var k in obj) { ...obj[k]... } repeatedly over many distinct object shapes (megamorphic property access), so the loop becomes hot.
  2. Ion compiles the loop and the OptimizeIteratorIndices pass rewrites obj[k] to use iterator indices, switching the runtime call to js::GetIteratorWithIndices.
  3. GetIteratorImpl enumerates an object that has an odd number of own enumerable data properties (e.g. 3) and a cacheable prototype chain of length 2 (Object.prototype), so the iterator is created with both an allocated PropertyIndex array (Flags::IndicesAllocated) and one proto-chain shape (protoShapeCount == 1).
  4. CreatePropertyIterator sizes the allocation via NumTrailingBytes and the NativeIterator constructor computes protoShapesEnd()/protoShapesBegin(): with IndicesAllocated and odd propertyCount, the GCPtr<Shape*> array starts at a 4-byte-aligned offset.
  5. The constructor executes new (protoShapesEnd()) GCPtr<Shape*>(shape) at the misaligned address (Iteration.cpp:922) and the iterator is subsequently loaded and GC-traced through that misaligned pointer — undefined behavior; faults on strict-alignment CPUs and is a confirmed UBSan alignment violation.

Steps to Reproduce

  1. Build the ASAN fuzzing shell, additionally compiling ONLY js/src/vm/Iteration.cpp with -fsanitize=alignment -fno-sanitize-recover=alignment (move it from UNIFIED_SOURCES to SOURCES in js/src/vm/moz.build and add the flags; the ASAN build already links the UBSan runtime via --enable-undefined-sanitizer). See moz.build.env-change.patch.
  2. Rebuild /firefox/obj-firefox-asan with /app/mozconfigs/mozconfig.linux.asan.fuzzing.
  3. Run: /firefox/obj-firefox-asan/dist/bin/js --fuzzing-safe --ion-iterator-indices=on testcase.js
  4. Observe the UndefinedBehaviorSanitizer abort at Iteration.cpp:922 reporting a constructor call on a 4-byte-misaligned address for type GCPtr<Shape*> (which requires 8-byte alignment), with stack NativeIterator::NativeIterator <- CreatePropertyIterator <- GetIteratorImpl.

Security Impact

  • Severity: Moderate
  • Attacker capability: Web content can deterministically drive the JS engine to construct and then load/GC-trace an 8-byte-aligned GCPtr<Shape*> at a 4-byte-aligned address inside a heap allocation. This is a memory-safety invariant violation (undefined behavior): on strict-alignment targets (e.g. 32-bit ARM/SPARC) the misaligned access traps (SIGBUS -> denial of service), and because the access is formally UB the compiler is entitled to assume the pointer is aligned, which can lead to miscompiled GC pointer reads/writes (e.g. atomic or vectorized accesses) and consequent heap corruption. On mainstream x86_64/aarch64 the misaligned scalar access happens to work, which constrains direct exploitability and is why the defect is latent/undisclosed.
  • Preconditions: Reachable from ordinary, unprivileged JavaScript (no special prefs or testing functions). Requires the Ion iterator-indices optimization to engage on a for (k in obj) obj[k] loop, which happens automatically for hot, megamorphic loops; the target object must have an odd number of own enumerable data properties and a cacheable prototype chain of length > 1.

ASAN Report

/firefox/js/src/vm/Iteration.cpp:922:32: runtime error: constructor call on misaligned address 0x75b466ae2724 for type 'GCPtr<Shape *>', which requires 8 byte alignment
0x75b466ae2724: note: pointer points here
  e4 e4 e4 e4 e4 e4 e4 e4  e4 e4 e4 e4 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
              ^
    #0 0x5620e82c4d0c in js::NativeIterator::NativeIterator(...) /firefox/js/src/vm/Iteration.cpp:922:9
    #1 0x5620e82cbfd7 in CreatePropertyIterator(...) /firefox/js/src/vm/Iteration.cpp:840:13
    #2 0x5620e82c67a8 in GetIteratorImpl(JSContext*, JS::Handle<JSObject*>, bool, bool) /firefox/js/src/vm/Iteration.cpp:1288:37
    #3 0x7503e4bd77fa  (<unknown module>)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /firefox/js/src/vm/Iteration.cpp:922:32
Attached file testcase.js —
Attached file run_command.txt —
Attached patch fix.patch — — Splinter Review
Attached file crash_stack.txt —
Group: core-security → javascript-core-security
See Also: → 2013993
Blocks: sm-security, GC
Severity: -- → S3
Priority: -- → P3

This was fixed in bug 2013993.

Status: UNCONFIRMED → RESOLVED
Closed: 4 months ago
Duplicate of bug: 2013993
Resolution: --- → DUPLICATE
Group: javascript-core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: