303015 2025-11-22 17:44:06 -0800 UB in Structure::flattenDictionaryStructure, member call on null pointer of type 'JSC::Butterfly' 2025-11-24 23:25:14 -0800 1 1 1 Unclassified WebKit JavaScriptCore WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 jarred webkit-unassigned contact syg webkit-bug-importer ysuzuki oldest_to_newest 2160881 0 jarred 2025-11-22 17:44:06 -0800 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(); ``` 2160882 1 jarred 2025-11-22 17:45:37 -0800 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) 2160887 2 contact 2025-11-22 18:33:57 -0800 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). 2161151 3 ap 2025-11-24 17:30:34 -0800 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. 2161179 4 sosuke 2025-11-24 20:51:09 -0800 Pull request: https://github.com/WebKit/WebKit/pull/54431 2161199 5 ews-feeder 2025-11-24 23:24:52 -0800 Committed 303524@main (8fbefe5287a8): <https://commits.webkit.org/303524@main> Reviewed commits have been landed. Closing PR #54431 and removing active labels. 2161200 6 webkit-bug-importer 2025-11-24 23:25:14 -0800 <rdar://problem/165384941>