Bug 250931 - Parser's PropertyNode should never be given an Identifier with a null impl.
Summary: Parser's PropertyNode should never be given an Identifier with a null impl.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-01-21 02:21 PST by Samuel Groß
Modified: 2023-01-22 08:45 PST (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Samuel Groß 2023-01-21 02:21:49 PST
The following testcase triggers an assertion failure in debug builds of JSC from current HEAD (and a nullptr deref in release builds):

    class C0 {
        static get c() {
        }
        static {
        }
    }
    // CRASH INFO
    // ==========
    // TERMSIG: 6
    // STDERR:
    // ASSERTION FAILED: !HashTranslator::equal(KeyTraits::emptyValue(), key)
    // /home/builder/webkit/FuzzBuild/Debug/WTF/Headers/wtf/HashTable.h(659) : void WTF::HashTable<WTF::UniquedStringImpl *, WTF::KeyValuePair<WTF::UniquedStringImpl *, std::pair<JSC::PropertyNode *, JSC::PropertyNode *>>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::UniquedStringImpl *, std::pair<JSC::PropertyNode *, JSC::PropertyNode *>>>, JSC::IdentifierRepHash, WTF::HashMap<WTF::UniquedStringImpl *, std::pair<JSC::PropertyNode *, JSC::PropertyNode *>, JSC::IdentifierRepHash>::KeyValuePairTraits, WTF::HashTraits<WTF::UniquedStringImpl *>>::checkKey(const T &) [Key = WTF::UniquedStringImpl *, Value = WTF::KeyValuePair<WTF::UniquedStringImpl *, std::pair<JSC::PropertyNode *, JSC::PropertyNode *>>, Extractor = WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::UniquedStringImpl *, std::pair<JSC::PropertyNode *, JSC::PropertyNode *>>>, HashFunctions = JSC::IdentifierRepHash, Traits = WTF::HashMap<WTF::UniquedStringImpl *, std::pair<JSC::PropertyNode *, JSC::PropertyNode *>, JSC::IdentifierRepHash>::KeyValuePairTraits, KeyTraits = WTF::HashTraits<WTF::UniquedStringImpl *>, HashTranslator = WTF::IdentityHashTranslator<WTF::HashMap<WTF::UniquedStringImpl *, std::pair<JSC::PropertyNode *, JSC::PropertyNode *>, JSC::IdentifierRepHash>::KeyValuePairTraits, JSC::IdentifierRepHash>, T = WTF::UniquedStringImpl *]
    // STDOUT:
    // ARGS: ./jsc/jsc --validateOptions=true --thresholdForJITSoon=10 --thresholdForJITAfterWarmUp=10 --thresholdForOptimizeAfterWarmUp=100 --thresholdForOptimizeAfterLongWarmUp=100 --thresholdForOptimizeSoon=100 --thresholdForFTLOptimizeAfterWarmUp=1000 --thresholdForFTLOptimizeSoon=1000 --validateBCE=true --reprl
    // EXECUTION TIME: 26 ms

I'm not sure if this assertion has any security implications (other than a nullptr deref) so I'm filing this as a security issue as a precaution. I also don't believe that Safari is currently affected (Version 16.2) as it doesn't yet seem to support class static initializers.
Comment 1 Radar WebKit Bug Importer 2023-01-21 02:22:01 PST
<rdar://problem/104507750>
Comment 2 Darin Adler 2023-01-21 07:36:08 PST
I'm pretty sure that the symptom here is indeed just a null dereference without deeper exploitable properties. When the empty value is used to look in the hash table, the code could incorrectly treat the hash table slot as non-empty, but next the key comparison will be done which will immediately result in a null dereference.

If I could see the rest of the backtrace I could quickly form more of an opinion on what the mistake is that leads to this. It's probably super-easy to fix.
Comment 3 Samuel Groß 2023-01-21 07:40:14 PST
Here's the full backtrace at the time of the assertion failure:

    #0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
    #1  0x00007ffff18add2f in __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78
    #2  0x00007ffff185eef2 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
    #3  0x00007ffff1849472 in __GI_abort () at ./stdlib/abort.c:79
    #4  0x00007ffff539a12b in WTFCrashWithInfo () at WTF/Headers/wtf/Assertions.h:754
    #5  0x00007ffff5badc6d in WTF::HashTable<WTF::UniquedStringImpl*, WTF::KeyValuePair<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*> > >, JSC::IdentifierRepHash, WTF::HashMap<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*>, JSC::IdentifierRepHash, WTF::HashTraits<WTF::UniquedStringImpl*>, WTF::HashTraits<std::pair<JSC::PropertyNode*, JSC::PropertyNode*> >, WTF::HashTableTraits>::KeyValuePairTraits, WTF::HashTraits<WTF::UniquedStringImpl*> >::checkKey<WTF::IdentityHashTranslator<WTF::HashMap<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*>, JSC::IdentifierRepHash, WTF::HashTraits<WTF::UniquedStringImpl*>, WTF::HashTraits<std::pair<JSC::PropertyNode*, JSC::PropertyNode*> >, WTF::HashTableTraits>::KeyValuePairTraits, JSC::IdentifierRepHash>, WTF::UniquedStringImpl*> (this=0x7fffffffae80, 
        key=@0x7fffffffae60: 0x0) at WTF/Headers/wtf/HashTable.h:659
    #6  0x00007ffff5baefb1 in WTF::HashTable<WTF::UniquedStringImpl*, WTF::KeyValuePair<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*> > >, JSC::IdentifierRepHash, WTF::HashMap<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*>, JSC::IdentifierRepHash, WTF::HashTraits<WTF::UniquedStringImpl*>, WTF::HashTraits<std::pair<JSC::PropertyNode*, JSC::PropertyNode*> >, WTF::HashTableTraits>::KeyValuePairTraits, WTF::HashTraits<WTF::UniquedStringImpl*> >::inlineLookup<WTF::IdentityHashTranslator<WTF::HashMap<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*>, JSC::IdentifierRepHash, WTF::HashTraits<WTF::UniquedStringImpl*>, WTF::HashTraits<std::pair<JSC::PropertyNode*, JSC::PropertyNode*> >, WTF::HashTableTraits>::KeyValuePairTraits, JSC::IdentifierRepHash>, WTF::UniquedStringImpl*> (this=0x7fffffffae80, 
        key=@0x7fffffffae60: 0x0) at WTF/Headers/wtf/HashTable.h:681
    #7  0x00007ffff5baef7d in WTF::HashTable<WTF::UniquedStringImpl*, WTF::KeyValuePair<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*> > >, JSC::IdentifierRepHash, WTF::HashMap<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*>, JSC::IdentifierRepHash, WTF::HashTraits<WTF::UniquedStringImpl*>, WTF::HashTraits<std::pair<JSC::PropertyNode*, JSC::PropertyNode*> >, WTF::HashTableTraits>::KeyValuePairTraits, WTF::HashTraits<WTF::UniquedStringImpl*> >::lookup<WTF::IdentityHashTranslator<WTF::HashMap<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*>, JSC::IdentifierRepHash, WTF::HashTraits<WTF::UniquedStringImpl*>, WTF::HashTraits<std::pair<JSC::PropertyNode*, JSC::PropertyNode*> >, WTF::HashTableTraits>::KeyValuePairTraits, JSC::IdentifierRepHash>, WTF::UniquedStringImpl*> (this=0x7fffffffae80, 
        key=@0x7fffffffae60: 0x0) at WTF/Headers/wtf/HashTable.h:673
    #8  0x00007ffff5baef38 in WTF::HashTable<WTF::UniquedStringImpl*, WTF::KeyValuePair<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*> > >, JSC::IdentifierRepHash, WTF::HashMap<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*>, JSC::IdentifierRepHash, WTF::HashTraits<WTF::UniquedStringImpl*>, WTF::HashTraits<std::pair<JSC::PropertyNode*, JSC::PropertyNode*> >, WTF::HashTableTraits>::KeyValuePairTraits, WTF::HashTraits<WTF::UniquedStringImpl*> >::contains<WTF::IdentityHashTranslator<WTF::HashMap<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*>, JSC::IdentifierRepHash, WTF::HashTraits<WTF::UniquedStringImpl*>, WTF::HashTraits<std::pair<JSC::PropertyNode*, JSC::PropertyNode*> >, WTF::HashTableTraits>::KeyValuePairTraits, JSC::IdentifierRepHash>, WTF::UniquedStringImpl*> (this=0x7fffffffae80, 
        key=@0x7fffffffae60: 0x0) at WTF/Headers/wtf/HashTable.h:1053
    #9  0x00007ffff5baeeed in WTF::HashTable<WTF::UniquedStringImpl*, WTF::KeyValuePair<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*> >, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*> > >, JSC::IdentifierRepHash, WTF::HashMap<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*>, JSC::IdentifierRepHash, WTF::HashTraits<WTF::UniquedStringImpl*>, WTF::HashTraits<std::pair<JSC::PropertyNode*, JSC::PropertyNode*> >, WTF::HashTableTraits>::KeyValuePairTraits, WTF::HashTraits<WTF::UniquedStringImpl*> >::contains (this=0x7fffffffae80, key=@0x7fffffffae60: 0x0) at WTF/Headers/wtf/HashTable.h:500
    #10 0x00007ffff5b09cfd in WTF::HashMap<WTF::UniquedStringImpl*, std::pair<JSC::PropertyNode*, JSC::PropertyNode*>, JSC::IdentifierRepHash, WTF::HashTraits<WTF::UniquedStringImpl*>, WTF::HashTraits<std::pair<JSC::PropertyNode*, JSC::PropertyNode*> >, WTF::HashTableTraits>::contains (this=0x7fffffffae80, key=@0x7fffffffae60: 0x0) at WTF/Headers/wtf/HashMap.h:323
    #11 0x00007ffff5ac67aa in JSC::PropertyListNode::emitBytecode (this=0x7fffa54a20e8, generator=..., dstOrConstructor=0x7fffe700c3ac, prototype=0x7fffe700c3b8, instanceFieldLocations=0x7fffffffb3b8, staticFieldLocations=0x7fffffffb3c8)
        at Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:681
    #12 0x00007ffff5b0cb70 in JSC::BytecodeGenerator::emitDefineClassElements (this=0x7fffe7045e80, n=0x7fffa54a20e8, constructor=0x7fffe700c3ac, prototype=0x7fffe700c3b8, instanceFieldLocations=..., staticFieldLocations=...)
        at Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:538
    #13 0x00007ffff5aeda81 in JSC::ClassExprNode::emitBytecode (this=0x7fffa54a21e8, generator=..., dst=0x0) at Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:5220
    #14 0x00007ffff5b0b579 in JSC::BytecodeGenerator::emitNodeInTailPosition (this=0x7fffe7045e80, dst=0x0, n=0x7fffa54a21e8) at Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:518
    #15 0x00007ffff5afbc75 in JSC::BytecodeGenerator::emitNode (this=0x7fffe7045e80, dst=0x0, n=0x7fffa54a21e8) at Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:507
    #16 0x00007ffff5ae4346 in JSC::AssignResolveNode::emitBytecode (this=0x7fffa54a22d8, generator=..., dst=0x0) at Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:3675
    #17 0x00007ffff5b0b579 in JSC::BytecodeGenerator::emitNodeInTailPosition (this=0x7fffe7045e80, dst=0x0, n=0x7fffa54a22d8) at Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:518
    #18 0x00007ffff5afbc75 in JSC::BytecodeGenerator::emitNode (this=0x7fffe7045e80, dst=0x0, n=0x7fffa54a22d8) at Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:507
    #19 0x00007ffff5b090a1 in JSC::BytecodeGenerator::emitNode (this=0x7fffe7045e80, n=0x7fffa54a22d8) at Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:523
    #20 0x00007ffff5aecfe5 in JSC::ClassDeclNode::emitBytecode (this=0x7fffa54a2330, generator=...) at Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:5136
    #21 0x00007ffff5b0ba85 in JSC::BytecodeGenerator::emitNodeInTailPosition (this=0x7fffe7045e80, dst=0x7fffe700c388, n=0x7fffa54a2330) at Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:486
    #22 0x00007ffff5b0b89c in JSC::SourceElements::emitBytecode (this=0x7fffa54a2000, generator=..., dst=0x7fffe700c388) at Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:3874
    #23 0x00007ffff5b0c207 in JSC::ScopeNode::emitStatementsBytecode (this=0x7fffe703ce80, generator=..., dst=0x7fffe700c388) at Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:4810
    #24 0x00007ffff5aeb0b8 in JSC::emitProgramNodeBytecode (generator=..., scopeNode=...) at Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:4820
    #25 0x00007ffff5aeafd1 in JSC::ProgramNode::emitBytecode (this=0x7fffe703ce80, generator=...) at Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:4830
    #26 0x00007ffff5aa5824 in JSC::BytecodeGenerator::generate (this=0x7fffe7045e80, size=@0x7fffffffbcdc: 32767) at Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:244
    #27 0x00007ffff6b0cf43 in JSC::BytecodeGenerator::generate<JSC::ProgramNode, JSC::UnlinkedProgramCodeBlock> (vm=..., node=0x7fffe703ce80, sourceCode=..., unlinkedCodeBlock=0x7fffe707d0e8, codeGenerationMode=..., parentScopeTDZVariables=..., 
        privateNameEnvironment=0x0) at Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h:389
    #28 0x00007ffff6b0eec7 in JSC::generateUnlinkedCodeBlockImpl<JSC::UnlinkedProgramCodeBlock, JSC::ProgramExecutable> (vm=..., source=..., strictMode=JSC::JSParserStrictMode::NotStrict, scriptMode=JSC::JSParserScriptMode::Classic, codeGenerationMode=..., 
        error=..., evalContextType=JSC::EvalContextType::None, derivedContextType=JSC::DerivedContextType::None, isArrowFunctionContext=false, variablesUnderTDZ=0x0, privateNameEnvironment=0x0, executable=0x7fffe7038308)
        at Source/JavaScriptCore/runtime/CodeCache.cpp:111
    #29 0x00007ffff6b0e109 in JSC::generateUnlinkedCodeBlock<JSC::UnlinkedProgramCodeBlock, JSC::ProgramExecutable> (vm=..., executable=0x7fffe7038308, source=..., strictMode=JSC::JSParserStrictMode::NotStrict, scriptMode=JSC::JSParserScriptMode::Classic, 
        codeGenerationMode=..., error=..., evalContextType=JSC::EvalContextType::None, variablesUnderTDZ=0x0, privateNameEnvironment=0x0) at Source/JavaScriptCore/runtime/CodeCache.cpp:122
    #30 0x00007ffff6b02373 in JSC::CodeCache::getUnlinkedGlobalCodeBlock<JSC::UnlinkedProgramCodeBlock, JSC::ProgramExecutable> (this=0x7fffe70242c0, vm=..., executable=0x7fffe7038308, source=..., strictMode=JSC::JSParserStrictMode::NotStrict, 
        scriptMode=JSC::JSParserScriptMode::Classic, codeGenerationMode=..., error=..., evalContextType=JSC::EvalContextType::None) at Source/JavaScriptCore/runtime/CodeCache.cpp:176
    #31 0x00007ffff6aff932 in JSC::CodeCache::getUnlinkedProgramCodeBlock (this=0x7fffe70242c0, vm=..., executable=0x7fffe7038308, source=..., strictMode=JSC::JSParserStrictMode::NotStrict, codeGenerationMode=..., error=...)
        at Source/JavaScriptCore/runtime/CodeCache.cpp:191
    #32 0x00007ffff6eef790 in JSC::ProgramExecutable::initializeGlobalProperties (this=0x7fffe7038308, vm=..., globalObject=0x7fffa541a068, scope=0x7fffe7020428)
        at Source/JavaScriptCore/runtime/ProgramExecutable.cpp:79
    #33 0x00007ffff6754213 in JSC::Interpreter::executeProgram (this=0x7fffa500c8c0, source=..., thisObj=0x7fffe7000fa8) at Source/JavaScriptCore/interpreter/Interpreter.cpp:993
    --Type <RET> for more, q to quit, c to continue without paging--
    #34 0x00007ffff6b2b875 in JSC::evaluate (globalObject=0x7fffa541a068, source=..., thisValue=..., returnedException=...) at Source/JavaScriptCore/runtime/Completion.cpp:137
    #35 0x00005555555d8347 in runWithOptions (globalObject=0x7fffa541a068, options=..., success=@0x7fffffffd663: true) at Source/JavaScriptCore/jsc.cpp:3593
    #36 0x00005555555bdfac in jscmain(int, char**)::$_11::operator()(JSC::VM&, GlobalObject*, bool&) const (this=0x7fffffffd730, vm=..., globalObject=0x7fffa541a068, success=@0x7fffffffd663: true)
        at Source/JavaScriptCore/jsc.cpp:4218
    #37 0x00005555555ba844 in runJSC<jscmain(int, char**)::$_11>(CommandLine const&, bool, jscmain(int, char**)::$_11 const&) (options=..., isWorker=false, func=...) at Source/JavaScriptCore/jsc.cpp:4018
    #38 0x00005555555b8ca3 in jscmain (argc=2, argv=0x7fffffffd8b8) at Source/JavaScriptCore/jsc.cpp:4211
    #39 0x00005555555b8a4d in main (argc=2, argv=0x7fffffffd8b8) at Source/JavaScriptCore/jsc.cpp:3360


Let me know if the backtrace for the nullptr deref crash in release builds would be useful as well!
Comment 4 Mark Lam 2023-01-21 19:32:29 PST
This issue is purely due to a bug of using a null string as an Identifier.  As a result, it triggers that ASSERT failure.  On a Release build, this is purely a null dereference.  There is no security issue here.
Comment 5 Mark Lam 2023-01-21 19:44:17 PST
Pull request: https://github.com/WebKit/WebKit/pull/8938
Comment 6 EWS 2023-01-22 08:45:20 PST
Committed 259187@main (90bd38013bce): <https://commits.webkit.org/259187@main>

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