Repository navigation
ALPN callback function sometimes leads to segfault in node.js >= 18.13.0 #47207
Description
Activity
And another one few second ago:
PID 8 received SIGSEGV for address: 0x400000001081 /srv/shorturl_redirector/node_modules/segfault-handler/build/Release/segfault-handler.node(+0x3236)[0x7fbc68045236] /lib/x86_64-linux-gnu/libpthread.so.0(+0x13140)[0x7fbc6ad44140] node(SSL_select_next_proto+0x4c)[0x17f0bfc] node[0xd243e0] node(tls_handle_alpn+0x53)[0x1834f83] node(tls_parse_all_extensions+0x143)[0x18176f3] node(tls_post_process_client_hello+0x70)[0x1835200] node[0x1822b74] node(ssl3_read_bytes+0x320)[0x1811050] node(ssl3_read+0x60)[0x17e0c40] node(SSL_read+0x87)[0x17ee3f7] node(_ZN4node6crypto7TLSWrap8ClearOutEv+0x77)[0xd2c8f7] node(_ZN4node6crypto7TLSWrap12OnStreamReadElRK8uv_buf_t+0xf8)[0xd2d5f8] node(_ZN4node15LibuvStreamWrap8OnUvReadElPK8uv_buf_t+0x89)[0xc6f959] node[0xc6fd68] node[0x1676f67] node[0x1677790] node[0x167d534] node(uv_run+0x14e)[0x166b95e] node(_ZN4node13SpinEventLoopEPNS_11EnvironmentE+0x14d)[0xabda2d] node(_ZN4node16NodeMainInstance3RunEv+0xf4)[0xbc1874] node(_ZN4node22LoadSnapshotDataAndRunEPPKNS_12SnapshotDataEPKNS_20InitializationResultE+0xb4)[0xb36434] node(_ZN4node5StartEiPPc+0x2df)[0xb3a02f] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xea)[0x7fbc6ab7fd0a] node(_start+0x2e)[0xaba37e] Segmentation fault (core dumped)It's 2AM and I finally extracted core dump from kubernetes. Does not tell much because I need debug symbols for node 18.15.0
(gdb) bt #0 0x00000000017f0bfc in SSL_select_next_proto () #1 0x0000000000d243e0 in node::crypto::(anonymous namespace)::SelectALPNCallback(ssl_st*, unsigned char const**, unsigned char*, unsigned char const*, unsigned int, void*) () #2 0x0000000001834f83 in tls_handle_alpn () #3 0x00000000018176f3 in tls_parse_all_extensions () #4 0x0000000001835200 in tls_post_process_client_hello () #5 0x0000000001822b74 in state_machine.part () #6 0x0000000001811050 in ssl3_read_bytes () #7 0x00000000017e0c40 in ssl3_read () #8 0x00000000017ee3f7 in SSL_read () #9 0x0000000000d2c8f7 in node::crypto::TLSWrap::ClearOut() () #10 0x0000000000d2d5f8 in node::crypto::TLSWrap::OnStreamRead(long, uv_buf_t const&) () #11 0x0000000000c6f959 in node::LibuvStreamWrap::OnUvRead(long, uv_buf_t const*) () #12 0x0000000000c6fd68 in node::LibuvStreamWrap::ReadStart()::{lambda(uv_stream_s*, long, uv_buf_t const*)#2}::_FUN(uv_stream_s*, long, uv_buf_t const*) () #13 0x0000000001676f67 in uv__read (stream=stream@entry=0x7f4994e92e90) at ../deps/uv/src/unix/stream.c:1201 #14 0x0000000001677790 in uv__stream_io (loop=<optimized out>, w=0x7f4994e92f18, events=1) at ../deps/uv/src/unix/stream.c:1270 #15 0x000000000167d534 in uv__io_poll (loop=loop@entry=0x526dfc0 <default_loop_struct>, timeout=<optimized out>) at ../deps/uv/src/unix/epoll.c:374 #16 0x000000000166b95e in uv_run (loop=0x526dfc0 <default_loop_struct>, mode=UV_RUN_DEFAULT) at ../deps/uv/src/unix/core.c:406 #17 0x0000000000abda2d in node::SpinEventLoop(node::Environment*) () #18 0x0000000000bc1874 in node::NodeMainInstance::Run() () #19 0x0000000000b36434 in node::LoadSnapshotDataAndRun(node::SnapshotData const**, node::InitializationResult const*) () #20 0x0000000000b3a02f in node::Start(int, char**) () #21 0x00007f4ee8745d0a in __libc_start_main (main=0xab1f70 <main>, argc=3, argv=0x7ffdb3a07ad8, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>,and
(gdb) info frame Stack level 1, frame at 0x7ffdb39ffe50: rip = 0xd243e0 in node::crypto::(anonymous namespace)::SelectALPNCallback(ssl_st*, unsigned char const**, unsigned char*, unsigned char const*, unsigned int, void*); saved rip = 0x1834f83 called by frame at 0x7ffdb39ffe90, caller of frame at 0x7ffdb39ffe40 Arglist at 0x7ffdb39ffe38, args: Locals at 0x7ffdb39ffe38, Previous frame's sp is 0x7ffdb39ffe50 Saved registers: rbp at 0x7ffdb39ffe40, rip at 0x7ffdb39ffe48A
disassembleandinfo registersof the top frame might be enough for me to tell what's going on, and maybe a hex dump of the top 32 or 64 bytes of the stack.Dump of assembler code for function SSL_select_next_proto: 0x00000000017f0bb0 <+0>: push %rbp 0x00000000017f0bb1 <+1>: mov %rsp,%rbp 0x00000000017f0bb4 <+4>: push %r15 0x00000000017f0bb6 <+6>: mov %r8,%r15 0x00000000017f0bb9 <+9>: push %r14 0x00000000017f0bbb <+11>: mov %r9d,%r14d 0x00000000017f0bbe <+14>: push %r13 0x00000000017f0bc0 <+16>: push %r12 0x00000000017f0bc2 <+18>: push %rbx 0x00000000017f0bc3 <+19>: sub $0x48,%rsp 0x00000000017f0bc7 <+23>: mov %rdi,-0x60(%rbp) 0x00000000017f0bcb <+27>: mov %rsi,-0x68(%rbp) 0x00000000017f0bcf <+31>: mov %rdx,-0x58(%rbp) 0x00000000017f0bd3 <+35>: mov %ecx,-0x48(%rbp) 0x00000000017f0bd6 <+38>: movl $0x0,-0x44(%rbp) 0x00000000017f0bdd <+45>: test %ecx,%ecx 0x00000000017f0bdf <+47>: je 0x17f0c90 <SSL_select_next_proto+224> 0x00000000017f0be5 <+53>: mov -0x44(%rbp),%eax 0x00000000017f0be8 <+56>: mov -0x58(%rbp),%rdi 0x00000000017f0bec <+60>: xor %r12d,%r12d 0x00000000017f0bef <+63>: mov %rax,%rcx 0x00000000017f0bf2 <+66>: add %rdi,%rax 0x00000000017f0bf5 <+69>: mov %rax,-0x50(%rbp) 0x00000000017f0bf9 <+73>: lea 0x1(%rcx),%ebx => 0x00000000017f0bfc <+76>: movzbl (%rax),%eax 0x00000000017f0bff <+79>: add %rdi,%rbx 0x00000000017f0c02 <+82>: mov %rax,%r13 0x00000000017f0c05 <+85>: mov %rax,-0x38(%rbp) 0x00000000017f0c09 <+89>: mov %rbx,-0x40(%rbp) 0x00000000017f0c0d <+93>: test %r14d,%r14d 0x00000000017f0c10 <+96>: jne 0x17f0c22 <SSL_select_next_proto+114> 0x00000000017f0c12 <+98>: jmp 0x17f0c78 <SSL_select_next_proto+200> 0x00000000017f0c14 <+100>: nopl 0x0(%rax) 0x00000000017f0c18 <+104>: lea 0x1(%r12,%rbx,1),%r12d 0x00000000017f0c1d <+109>: cmp %r12d,%r14d 0x00000000017f0c20 <+112>: jbe 0x17f0c78 <SSL_select_next_proto+200> 0x00000000017f0c22 <+114>: mov %r12d,%eax 0x00000000017f0c25 <+117>: movzbl (%r15,%rax,1),%ebx 0x00000000017f0c2a <+122>: cmp %r13b,%bl 0x00000000017f0c2d <+125>: jne 0x17f0c18 <SSL_select_next_proto+104> 0x00000000017f0c2f <+127>: lea 0x1(%r12),%esi 0x00000000017f0c34 <+132>: mov -0x38(%rbp),%rdx 0x00000000017f0c38 <+136>: mov -0x40(%rbp),%rdi 0x00000000017f0c3c <+140>: add %r15,%rsi 0x00000000017f0c3f <+143>: call 0xa84da0 <memcmp@plt> 0x00000000017f0c44 <+148>: test %eax,%eax(gdb) info registers rax 0x75684f4f7156766e 8460099102637979246 rbx 0x1 1 rcx 0x0 0 rdx 0x75684f4f7156766e 8460099102637979246 rsi 0x7ffdb39ffe57 140727617060439 rdi 0x75684f4f7156766e 8460099102637979246 rbp 0x7ffdb39ffe30 0x7ffdb39ffe30 rsp 0x7ffdb39ffdc0 0x7ffdb39ffdc0 r8 0x7f499574c150 139954016797008 r9 0xc 12 r10 0x75684f4f7156766e 8460099102637979246 r11 0x27ab84dcf749 43617621964617 r12 0x0 0 r13 0x80 128 r14 0xc 12 r15 0x7f499574c150 139954016797008 rip 0x17f0bfc 0x17f0bfc <SSL_select_next_proto+76> eflags 0x10202 [ IF RF ] cs 0x33 51 ss 0x2b 43 ds 0x0 0 es 0x0 0 fs 0x0 0 gs 0x0 0 k0 0xfefef000 4278120448 k1 0xfc000000 4227858432 k2 0xffffffff 4294967295 k3 0xffffffff 4294967295 k4 0xffffffff 4294967295 k5 0xffffffff 4294967295 k6 0xffffffff 4294967295 k7 0x7 7ALPN callback
(gdb) info frame Stack level 1, frame at 0x7ffdb39ffe50: rip = 0xd243e0 in node::crypto::(anonymous namespace)::SelectALPNCallback(ssl_st*, unsigned char const**, unsigned char*, unsigned char const*, unsigned int, void*); saved rip = 0x1834f83 called by frame at 0x7ffdb39ffe90, caller of frame at 0x7ffdb39ffe40 Arglist at 0x7ffdb39ffe38, args: Locals at 0x7ffdb39ffe38, Previous frame's sp is 0x7ffdb39ffe50 Saved registers: rbp at 0x7ffdb39ffe40, rip at 0x7ffdb39ffe48 (gdb) x/8xw 0x7ffdb39ffe38 0x7ffdb39ffe38: 0x00d243e0 0x00000000 0xb39ffe80 0x00007ffd 0x7ffdb39ffe48: 0x01834f83 0x00000000 0x0000001c 0x00000000 (gdb)OpenSSL:
(gdb) info frame Stack level 0, frame at 0x7ffdb39ffe40: rip = 0x17f0bfc in SSL_select_next_proto; saved rip = 0xd243e0 called by frame at 0x7ffdb39ffe50 Arglist at 0x7ffdb39ffe30, args: Locals at 0x7ffdb39ffe30, Previous frame's sp is 0x7ffdb39ffe40 Saved registers: rbx at 0x7ffdb39ffe08, rbp at 0x7ffdb39ffe30, r12 at 0x7ffdb39ffe10, r13 at 0x7ffdb39ffe18, r14 at 0x7ffdb39ffe20, r15 at 0x7ffdb39ffe28, rip at 0x7ffdb39ffe38 (gdb) x/16wx 0x7ffdb39ffe30 0x7ffdb39ffe30: 0xb39ffe40 0x00007ffd 0x00d243e0 0x00000000 0x7ffdb39ffe40: 0xb39ffe80 0x00007ffd 0x01834f83 0x00000000 0x7ffdb39ffe50: 0x0000001c 0x00000000 0x00000000 0x00000000 0x7ffdb39ffe60: 0x95ca0898 0x00007f49 0x95ca09d8 0x00007f49I can't be 100% sure because the data is buried deeper in the stack than I anticipated (isn't it always like that?) but it looks like the ALPN string from the ClientHello packet is zero-sized.
SSL_select_next_proto() selects the first item from the client's string if there's no matching entry in the server's string but yeah, that won't work if the client's string is empty.
Do you have the opportunity to try out a patch locally? Does this one-liner fix it?
diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc index f14adec767a..fea3186c17e 100644 --- a/src/crypto/crypto_tls.cc +++ b/src/crypto/crypto_tls.cc @@ -225,6 +225,8 @@ int SelectALPNCallback( const unsigned char* in, unsigned int inlen, void* arg) { + if (inlen == 0) return SSL_TLSEXT_ERR_ALERT_FATAL; + TLSWrap* w = static_cast<TLSWrap*>(arg); const std::vector<unsigned char>& alpn_protos = w->alpn_protos_;
Reacted by skgith1No, but I'll try to patch openssl locally to try to reproduce this error on my dev end. If it is true it looks like a possibility DoS attack on node.js
Reacted by skgith1Found another one, tls client:
> s = tls.connect('short.io', 443, {ALPNProtocols: [""]}) node[138836]: ../src/crypto/crypto_tls.cc:1533:static void node::crypto::TLSWrap::SetALPNProtocols(const v8::FunctionCallbackInfo<v8::Value>&): Assertion `SetALPN(w->ssl_, args[0])' failed. 1: 0xb06730 node::Abort() [node] 2: 0xb067ae [node] 3: 0xca27ea node::crypto::TLSWrap::SetALPNProtocols(v8::FunctionCallbackInfo<v8::Value> const&) [node] 4: 0xd3e33e [node] 5: 0xd3f75f v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [node] 6: 0x15da1b9 [node]I can't be 100% sure because the data is buried deeper in the stack than I anticipated (isn't it always like that?) but it looks like the ALPN string from the ClientHello packet is zero-sized.
SSL_select_next_proto() selects the first item from the client's string if there's no matching entry in the server's string but yeah, that won't work if the client's string is empty.
Do you have the opportunity to try out a patch locally? Does this one-liner fix it?
diff --git a/src/crypto/crypto_tls.cc b/src/crypto/crypto_tls.cc index f14adec767a..fea3186c17e 100644 --- a/src/crypto/crypto_tls.cc +++ b/src/crypto/crypto_tls.cc @@ -225,6 +225,8 @@ int SelectALPNCallback( const unsigned char* in, unsigned int inlen, void* arg) { + if (inlen == 0) return SSL_TLSEXT_ERR_ALERT_FATAL; + TLSWrap* w = static_cast<TLSWrap*>(arg); const std::vector<unsigned char>& alpn_protos = w->alpn_protos_;
inlen equal to zero does not cause any issues, the problem is somewhere in out variable I think. Instead of prod I'm trying to reproduce it in this simple program:
#include <iostream> #include <openssl/ssl.h> using namespace std; int main() { unsigned char *out; unsigned char outlen; const char *server = "\x08http/1.1"; const char *client = "\x08http/1.1"; cout << strlen(server) << endl; int status = SSL_select_next_proto(&out, &outlen, (const unsigned char*)server, strlen(server), (const unsigned char*)client, strlen(client)); cout << status << endl; }
UPD: Setting
ALPNProtocols: []in createServer solves the issue for meUPD: Setting ALPNProtocols: [] in createServer solves the issue for me
What was it set to before? Which createServer method are you using (net/http/https/http2)?
it is https.createServer(), default value is
['http/1.1']FWIW, I've not been able to reproduce the crash (so far at least.)
I tried crafting a ClientHello with an empty ALPN extension record but I get back a handshake_failure alert and the connection is subsequently closed. No crash.
I tried various combinations of invalid ALPN extensions last week and also wasn't able to reproduce a crash on the server side. Is there any chance you might be able to share a traffic dump of just the TLS handshake that causes the server to crash @gugu?
32 remaining items
We can confirm that this issue still exists on node v18.18.0 and node v20.8.0.
We can also confirm that setting ALPNProtocols: [] does NOT resolve the issue.
About 1 of 2M HTTP requests in a representative production environment crashes the node.js process. Our production workload includes HTTP connection from almost all active browser versions. We do not believe this are malicious attempts as the failure rate follows seasonal patterns so its just a function of how many connections are made overall.
Reacted by Gilad Peleg- added a commit that references this issue
on Oct 4, 2023 - added a commit that references this issue
on Oct 28, 2023 - added a commit that references this issue
on Nov 11, 2023 We can confirm that the fix resolves this crash in node 20.8.0.
For posterity: the fix hasn't been released in v20.x yet. v20.10.0 is about to be released and contains the fix.
@bnoordhuis are there plans to backport this to 18.x?
Reacted by Brian Hill- added a commit that references this issue
on Apr 15, 2024 - added 2 commits that reference this issue
on Apr 25, 2024 We are using v20.11.0. The error is still present.
I confirm this bug is present with Node 18.18.2 and can be reproduced with 60% probability on the production system where there is quite a heavy load. I cannot reproduce it at all on the pre-production system which is almost identical.
Version
18.15.0
Platform
Linux 2a53a1799e0b 5.15.0-67-generic #74-Ubuntu SMP Wed Feb 22 14:14:39 UTC 2023 x86_64 GNU/Linux
Subsystem
tls
What steps will reproduce the bug?
I see this from logs of the my node.js server. I did not find a way to reproduce yet, and need some help with that. I've attached stacktrace, but did not yet found ALPN header value, which causes this error. According to my investigations, function
SelectALPNCallbackcallsSSL_select_next_protowith NULL instead of correct pointer. Looks like some maliciously crafted ALPN header can lead to such errorHow often does it reproduce? Is there a required condition?
It is a rare case I capture from logs (around 1 req/million). I can add some code to get more information about the bug, but don't know what to do
What is the expected behavior? Why is that the expected behavior?
Do not produce segfault
What do you see instead?
Segfoult with stacktrace:
Additional information
No response