Bug 303015 – UB in Structure::flattenDictionaryStructure, member call on null pointer of type 'JSC::Butterfly'

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Jarred Sumner]

Note: the below is written by Claude when prompted to investigate why the below reproduction causes a UBSan crash.

`Structure::flattenDictionaryStructure` in `Source/JavaScriptCore/runtime/Structure.cpp` dereferences a potentially null `Butterfly*` pointer without checking for null first, causing undefined behavior detected by UBSan.

## Minimal Reproduction

```javascript
const v3 = {
  p() {
    try {
      this.p();
    } catch (e) {}
    delete this.g;
    this.g = this;
  },
};
v3.p();
v3.p();
```

### Variations that affect the crash:

| Code                         | Crashes? |
| ---------------------------- | -------- |
| Original (just `p()` method) | Yes      |
| Add `q: 1` property          | No       |
| Add both `g: 1` and `q: 1`   | Yes      |
| Remove one `v3.p()` call     | No       |

## Environment

- **Upstream WebKit commit**: `e9aa5f3ee4abb3a80d1bf589d2630633171848dd` (approximately 300173@main, late September 2025)
- **Build type**: Debug with sanitizers (`-DENABLE_SANITIZERS="address,undefined"`)
- **Platform**: Linux aarch64
- **Compiler**: Clang 19

## UBSan Error Output

```
/path/to/JavaScriptCore/Butterfly.h:182:21: runtime error: member call on null pointer of type 'JSC::Butterfly *'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior Butterfly.h:182:21
```

## Full Stack Trace

```
#0 0x000008676e6c in JSC::Butterfly::indexingHeader() Butterfly.h:182
#1 0x000012b10744 in JSC::Structure::flattenDictionaryStructure(JSC::VM&, JSC::JSObject*) Structure.cpp:1042
#2 0x00000ee7cc00 in JSC::JSObject::flattenDictionaryObject(JSC::VM&) JSObject.h:818
#3 0x00000ee7ad04 in JSC::actionForCell(JSC::VM&, JSC::JSCell*) Repatch.cpp:351
#4 0x00000ee51f74 in JSC::tryCacheGetBy(JSC::JSGlobalObject*, JSC::CodeBlock*, JSC::JSValue, JSC::CacheableIdentifier, JSC::PropertySlot const&, JSC::StructureStubInfo&, JSC::GetByKind) Repatch.cpp:525
#5 0x00000ee50ab0 in JSC::repatchGetBy(JSC::JSGlobalObject*, JSC::CodeBlock*, JSC::JSValue, JSC::CacheableIdentifier, JSC::PropertySlot const&, JSC::StructureStubInfo&, JSC::GetByKind) Repatch.cpp:693
#6 0x0000117be8e8 in operationGetByIdOptimize::$_0::operator()(bool, JSC::PropertySlot&) const JITOperations.cpp:560
#7 0x0000117be22c in JSC::JSValue::getPropertySlot<...>(...) JSCJSValueInlines.h:1092
#8 0x000011707be0 in JSC::JSValue::getPropertySlot<...>(...) JSCJSValueInlines.h:1083
#9 0x000011707860 in operationGetByIdOptimize JITOperations.cpp:560
#10 0xe837ec000298 (<JIT code>)
#11 0x00000e779988 in llint_op_call_ignore_result LowLevelInterpreter.cpp
#12 0x00000e74ee60 in llint_call_javascript LowLevelInterpreter.cpp
#13 0x000011537e64 in JSC::Interpreter::executeModuleProgram(...) Interpreter.cpp:1784
#14 0x0000124187dc in JSC::JSModuleRecord::evaluate(...) JSModuleRecord.cpp:323
#15 0x000011c880ec in JSC::AbstractModuleRecord::evaluate(...) AbstractModuleRecord.cpp:869
#16 0x00001240ced0 in JSC::JSModuleLoader::evaluateNonVirtual(...) JSModuleLoader.cpp:308
...
```

## Root Cause Analysis

### The Bug Location

In `Source/JavaScriptCore/runtime/Structure.cpp`, function `flattenDictionaryStructure`, around line 1040-1046:

```cpp
if (isUncacheableDictionary()) {
    // ... earlier code ...

    Butterfly* butterfly = object->butterfly();
    size_t preCapacity = butterfly->indexingHeader()->preCapacity(this);  // BUG: no null check
    void* base = butterfly->base(preCapacity, beforeOutOfLineCapacity);
    void* startOfPropertyStorageSlots = reinterpret_cast<EncodedJSValue*>(base) + preCapacity;
    gcSafeZeroMemory(static_cast<JSValue*>(startOfPropertyStorageSlots), (beforeOutOfLineCapacity - outOfLineSize()) * sizeof(EncodedJSValue));
    checkOffsetConsistency();
}
```

### Why This Happens

1. The object becomes an **uncacheable dictionary** due to repeated property deletion and addition (`delete this.g; this.g = this;` in a recursive loop that hits stack overflow)

2. When the JIT tries to cache property access on the second `v3.p()` call, it calls `actionForCell()` in `Repatch.cpp`:

```cpp
// Repatch.cpp:347-352
if (structure->isUncacheableDictionary()) {
    if (structure->hasBeenFlattenedBefore())
        return GiveUpOnCache;
    // Flattening could have changed the offset, so return early for another try.
    asObject(cell)->flattenDictionaryObject(vm);
    return RetryCacheLater;
}
```

3. `flattenDictionaryObject` calls `flattenDictionaryStructure`, which assumes that if `beforeOutOfLineCapacity > 0`, then the object must have a butterfly. However, this assumption is incorrect.

4. The Structure's `outOfLineCapacity()` is calculated from `maxOffset()`, which reflects the structure's property layout, but the **object itself may not have a butterfly allocated** if all its properties fit inline.

### The Inconsistency

- `beforeOutOfLineCapacity = this->outOfLineCapacity()` returns a non-zero value based on the Structure's metadata
- But `object->butterfly()` returns `nullptr` because the object's actual properties are stored inline
- The code then dereferences the null butterfly pointer

## Suggested Fix

Add a null check before accessing the butterfly:

```cpp
Butterfly* butterfly = object->butterfly();
if (butterfly) {
    size_t preCapacity = butterfly->indexingHeader()->preCapacity(this);
    void* base = butterfly->base(preCapacity, beforeOutOfLineCapacity);
    void* startOfPropertyStorageSlots = reinterpret_cast<EncodedJSValue*>(base) + preCapacity;
    gcSafeZeroMemory(static_cast<JSValue*>(startOfPropertyStorageSlots), (beforeOutOfLineCapacity - outOfLineSize()) * sizeof(EncodedJSValue));
}
```

Note: The code later at line 1053 already performs a null check on the butterfly:

```cpp
if (object->butterfly() && beforeOutOfLineCapacity != afterOutOfLineCapacity) {
```

This suggests the null check was intended but missed in the earlier code block.

## Additional Context

### Why jsc Shell May Not Reproduce

The standalone `jsc` shell may not reproduce this issue because:

1. Different object creation patterns
2. Different module/script evaluation paths
3. The specific sequence of structure transitions may differ

The bug was discovered in Bun's integration with JSC, where the module evaluation path creates objects that end up with this specific state.

### Impact

- **Debug builds with UBSan**: Crashes with abort
- **Release builds**: Undefined behavior (likely no visible impact due to how the memory happens to be laid out, but technically UB)

## Related Code References

- `Structure::flattenDictionaryStructure`: Structure.cpp:990-1080
- `JSObject::flattenDictionaryObject`: JSObject.h:816-819
- `actionForCell`: Repatch.cpp:338-360
- `Butterfly::indexingHeader`: Butterfly.h:182
- `Structure::outOfLineCapacity`: Structure.h:599-601

## Test Case for WebKit Test Suite

```javascript
//@ runDefault

function testFlattenDictionaryWithNullButterfly() {
  const obj = {
    recurse() {
      try {
        this.recurse();
      } catch (e) {}
      delete this.dynamicProp;
      this.dynamicProp = this;
    },
  };

  // First call creates the dictionary structure via repeated delete/add
  obj.recurse();

  // Second call triggers IC caching which calls flattenDictionaryStructure
  obj.recurse();
}

testFlattenDictionaryWithNullButterfly();
```

[Jarred Sumner]

I suspect it didn't reproduce in our local `jsc` shell and only in the debug build of bun with asan & ubsan because we were not building the `jsc` shell with ubsan enabled (only ASAN and LSAN)

I can confirm that this issue can be reproduced with a UBsan-enabled build of the JSC shell.

Given this code:

    const v3 = {
      p() {
        try {
          this.p();
        } catch (e) {}
        delete this.g;
        this.g = this;
      },
    };
    v3.p();
    v3.p();

`jsc` reports these errors:

> /tmp/webkit/Source/JavaScriptCore/runtime/Structure.cpp:1042:41: runtime error: member call on null pointer of type 'JSC::Butterfly'
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /tmp/webkit/Source/JavaScriptCore/runtime/Structure.cpp:1042:41
> /tmp/webkit/Source/JavaScriptCore/runtime/Butterfly.h:182:21: runtime error: member call on null pointer of type 'JSC::Butterfly *'
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /tmp/webkit/Source/JavaScriptCore/runtime/Butterfly.h:182:21
> /tmp/webkit/Source/JavaScriptCore/runtime/IndexingHeader.h:74:61: runtime error: applying non-zero offset 18446744073709551608 to null pointer
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /tmp/webkit/Source/JavaScriptCore/runtime/IndexingHeader.h:74:61
> /tmp/webkit/Source/JavaScriptCore/runtime/Structure.cpp:1043:33: runtime error: member call on null pointer of type 'JSC::Butterfly'
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /tmp/webkit/Source/JavaScriptCore/runtime/Structure.cpp:1043:33
> /tmp/webkit/Source/JavaScriptCore/runtime/Butterfly.h:222:11: runtime error: member call on null pointer of type 'JSC::Butterfly *'
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /tmp/webkit/Source/JavaScriptCore/runtime/Butterfly.h:222:11
> /tmp/webkit/Source/JavaScriptCore/runtime/Butterfly.h:184:21: runtime error: member call on null pointer of type 'JSC::Butterfly *'
> SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /tmp/webkit/Source/JavaScriptCore/runtime/Butterfly.h:184:21
If one of the calls to `v3.p()` is removed, these errors no longer appear.

Note: this is using Bun's fork of WebKit (https://github.com/oven-sh/WebKit, commit c3e10aa).

[Alexey Proskuryakov]

Thank you for the report! I don't think that it's saying that there is a _potential_ null pointer dereference. It's saying that there is an actual method call being made on a null object pointer here:

        size_t preCapacity = butterfly->indexingHeader()->preCapacity(this);

It so happens that 0->indexingHeader()->preCapacity(this) doesn't crash in practice, but it certainly is UB.

[Sosuke Suzuki]

Pull request: https://github.com/WebKit/WebKit/pull/54431

[EWS]

Committed 303524@main (8fbefe5287a8): <https://commits.webkit.org/303524@main>

Reviewed commits have been landed. Closing PR #54431 and removing active labels.

[Radar WebKit Bug Importer]

<rdar://problem/165384941>