Created attachment 379724 [details] poc program Hi, during testing, I found this crash input. I tested it with commit ca690fd881e742d539636921af00cf8bc2e3987e in Linux. Current PoC triggers NULL dereference. I don't know this can cause serious security problems like code execution, but I filed it as security at least it can cause some memory corruption. Here is a stack trace from gdb. ``` Thread 1 "jsc" received signal SIGSEGV, Segmentation fault. [----------------------------------registers-----------------------------------] RAX: 0x0 RBX: 0x0 RCX: 0x0 RDX: 0x8 RSI: 0x0 RDI: 0x0 RBP: 0x7fffffffbc60 --> 0x7fffffffbce0 --> 0x7fffffffbd30 --> 0x7fffffffbda0 --> 0x7fffffffbe20 --> 0x7fffffffbef0 --> 0x7fffffffd530 --> 0x7fffffffd640 --> 0x7fffffffd9b0 --> 0x7fffffffd9e0 --> 0x7fffffffdc30 --> 0x7fffffffdce0 --> 0x7ff fffffdd10 --> 0x1cf0c70 (<__libc_csu_init>: push r15) RSP: 0x7fffffffbc60 --> 0x7fffffffbce0 --> 0x7fffffffbd30 --> 0x7fffffffbda0 --> 0x7fffffffbe20 --> 0x7fffffffbef0 --> 0x7fffffffd530 --> 0x7fffffffd640 --> 0x7fffffffd9b0 --> 0x7fffffffd9e0 --> 0x7fffffffdc30 --> 0x7fffffffdce0 --> 0x7ff fffffdd10 --> 0x1cf0c70 (<__libc_csu_init>: push r15) RIP: 0x421f4c (<WTF::StringImpl::length() const+12>: mov eax,DWORD PTR [rdi+0x4]) R8 : 0x0 R9 : 0x7ffff7ffc680 --> 0x0 R10: 0x7ffff7dd5000 --> 0x10102464c457f R11: 0x7ffff4e89260 --> 0x400000 --> 0x10102464c457f R12: 0x7fffb3bfad10 --> 0x68006800680068 ('h') R13: 0x7ffff3fcccc0 --> 0x89a9fbfa41fb3635 R14: 0xfffe000000000000 R15: 0xfffe000000000002 EFLAGS: 0x10206 (carry PARITY adjust zero sign trap INTERRUPT direction overflow) [-------------------------------------code-------------------------------------] 0x421f41 <WTF::StringImpl::length() const+1>: mov rbp,rsp 0x421f44 <WTF::StringImpl::length() const+4>: mov QWORD PTR [rbp-0x8],rdi 0x421f48 <WTF::StringImpl::length() const+8>: mov rdi,QWORD PTR [rbp-0x8] => 0x421f4c <WTF::StringImpl::length() const+12>: mov eax,DWORD PTR [rdi+0x4] 0x421f4f <WTF::StringImpl::length() const+15>: pop rbp 0x421f50 <WTF::StringImpl::length() const+16>: ret 0x421f51: data16 data16 data16 data16 data16 nop WORD PTR cs:[rax+rax*1+0x0] 0x421f60 <WTF::Ref<WTF::AtomStringImpl, WTF::DumbPtrTraits<WTF::AtomStringImpl> >::Ref(WTF::AtomStringImpl&)>: push rbp Stopped reason: SIGSEGV WTF::StringImpl::length (this=0x0) at DerivedSources/ForwardingHeaders/wtf/text/StringImpl.h:274 274 unsigned length() const { return m_length; } gdb-peda$ bt #0 WTF::StringImpl::length (this=0x0) at DerivedSources/ForwardingHeaders/wtf/text/StringImpl.h:274 #1 0x0000000000fbe34d in operationSwitchCharWithUnknownKeyType (exec=0x7fffffffbd30, encodedKey=0x7fffb3998000, tableIndex=0x0) at ../../Source/JavaScriptCore/jit/JITOperations.cpp:2306 #2 0x00007fffb3eff998 in ?? () #3 0x00007fffb3998000 in ?? () #4 0x00007fffb39e4000 in ?? () #5 0x00007fffb39d4000 in ?? () #6 0x00007fffb39d4000 in ?? () #7 0x00007fffb3bfad10 in ?? () #8 0x00007ffff3fcccc0 in ?? () #9 0xfffe000000000000 in ?? () #10 0xfffe000000000002 in ?? () #11 0x00007fffffffbda0 in ?? () #12 0x000000000104faf4 in llint_op_call () #13 0x00007fffb39a3d40 in ?? () #14 0x00007fffb39ac030 in ?? () #15 0x0000001600000001 in ?? () #16 0x00007fffb39e4000 in ?? () #17 0x00007fffb39ac030 in ?? () #18 0x000000000000000a in ?? () #19 0x00007fffb39d4000 in ?? () #20 0x00007fffb39d4000 in ?? () #21 0x00000000004068c0 in ?? () #22 0x00007fffffffddf0 in ?? () #23 0x0000000000000000 in ?? () ```
<rdar://problem/55782280>
Created attachment 381030 [details] proposed patch.
@Insu, thanks for the bug report. This is just a null dereference bug due to a missing OOME check.
Comment on attachment 381030 [details] proposed patch. r=me
Comment on attachment 381030 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=381030&action=review > Source/JavaScriptCore/jit/JITOperations.cpp:2317 > - StringImpl* value = asString(key)->value(exec).impl(); > - if (value->length() == 1) > + JSString* string = asString(key); > + if (string->length() == 1) { > + StringImpl* value = string->value(exec).impl(); > result = jumpTable.ctiForValue((*value)[0]).executableAddress(); > + } We can create arbitrary rope with length = 1, including substring IIRC. So, let's just check exception. And ensure DFG doesGC and mayExit are saying the right thing.
Created attachment 381034 [details] propose patch.
Comment on attachment 381034 [details] propose patch. View in context: https://bugs.webkit.org/attachment.cgi?id=381034&action=review r=me with nit. > Source/JavaScriptCore/jit/JITOperations.cpp:2316 > + StringImpl* value = string->value(exec).impl(); I suggest doing String value = string->value(exec); RETURN_IF_EXCEPTION(throwScope, nullptr); Use value[0] Or value.impl()[0] To make sure that string is live (I think this case is OK, but it is still better I think).
Created attachment 381042 [details] proposed patch.
Comment on attachment 381042 [details] proposed patch. r=me
Thanks for the review. Landed in r251178: <http://trac.webkit.org/r251178>.