Skip to content

ALPN callback function sometimes leads to segfault in node.js >= 18.13.0 #47207

Closed
#49635
Closed
ALPN callback function sometimes leads to segfault in node.js >= 18.13.0#47207
#49635
Labels
tlsIssues and PRs related to the tls subsystem.

Description

@gugu
gugu
opened on Mar 21, 2023

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 SelectALPNCallback calls SSL_select_next_proto with NULL instead of correct pointer. Looks like some maliciously crafted ALPN header can lead to such error

How 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:

PID 9 received SIGSEGV for address: 0x0
/srv/shorturl_redirector/node_modules/segfault-handler/build/Release/segfault-handler.node(+0x3236)[0x7f86dc0d0236]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x13140)[0x7f86dedcf140]
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)[0x7f86dec0ad0a]
node(_start+0x2e)[0xaba37e]
Segmentation fault (core dumped)

Additional information

No response

Activity

  1. gugu commented on Mar 21, 2023

    @gugu
    gugu
    on Mar 21, 2023
    Author

    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)
  2. Imagebnoordhuis
    added
    tlsIssues and PRs related to the tls subsystem.
    on Mar 22, 2023

  3. gugu commented on Mar 22, 2023

    @gugu
    gugu
    on Mar 22, 2023
    Author

    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 0x7ffdb39ffe48

  4. bnoordhuis commented on Mar 23, 2023

    @bnoordhuis
    bnoordhuis
    on Mar 23, 2023
    Member

    A disassemble and info registers of 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.

  5. gugu commented on Mar 23, 2023

    @gugu
    gugu
    on Mar 23, 2023
    Author
    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                 7

  6. gugu commented on Mar 23, 2023

    @gugu
    gugu
    on Mar 23, 2023
    Author

    ALPN 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)

  7. gugu commented on Mar 23, 2023

    @gugu
    gugu
    on Mar 23, 2023
    Author

    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      0x00007f49

  8. bnoordhuis commented on Mar 23, 2023

    @bnoordhuis
    bnoordhuis
    on Mar 23, 2023
    Member

    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_;

  9. gugu commented on Mar 23, 2023

    @gugu
    gugu
    on Mar 23, 2023
    Author

    No, 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

  10. gugu commented on Mar 23, 2023

    @gugu
    gugu
    on Mar 23, 2023
    Author

    Found 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]

  11. gugu commented on Mar 23, 2023

    @gugu
    gugu
    on Mar 23, 2023
    Author

    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 me

  12. bnoordhuis commented on Mar 24, 2023

    @bnoordhuis
    bnoordhuis
    on Mar 24, 2023
    Member

    UPD: 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)?

  13. gugu commented on Mar 24, 2023

    @gugu
    gugu
    on Mar 24, 2023
    Author

    it is https.createServer(), default value is ['http/1.1']

  14. bnoordhuis commented on Mar 27, 2023

    @bnoordhuis
    bnoordhuis
    on Mar 27, 2023
    Member

    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.

  15. tniessen commented on Mar 27, 2023

    @tniessen
    tniessen
    on Mar 27, 2023
    Member

    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?

  16. 32 remaining items

  17. sempi commented on Sep 30, 2023

    @sempi
    sempi
    on Sep 30, 2023

    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.

  18. sempi commented on Oct 1, 2023

    @sempi
    sempi
    on Oct 1, 2023

    We can confirm that the fix resolves this crash in node 20.8.0.

    #49635

  19. Imagenodejs-github-bot
    closed this as completedin #49635on Oct 4, 2023
  20. Imagenodejs-github-bot
    added a commit that references this issue on Oct 4, 2023

    src: fix TLSWrap lifetime bug in ALPN callback

    1643adf
  21. Imagetargos
    added a commit that references this issue on Oct 28, 2023

    src: fix TLSWrap lifetime bug in ALPN callback

    62e2e59
  22. Imagetargos
    added a commit that references this issue on Nov 11, 2023

    src: fix TLSWrap lifetime bug in ALPN callback

    47164e2

  23. bnoordhuis commented on Nov 12, 2023

    @bnoordhuis
    bnoordhuis
    on Nov 12, 2023
    Member

    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.

  24. nromito commented on Nov 16, 2023

    @nromito
    nromito
    on Nov 16, 2023

    @bnoordhuis are there plans to backport this to 18.x?

  25. Imagedebadree25
    added a commit that references this issue on Apr 15, 2024

    src: fix TLSWrap lifetime bug in ALPN callback

    6761dc2
  26. Imagesercher
    added 2 commits that reference this issue on Apr 25, 2024

    src: fix TLSWrap lifetime bug in ALPN callback

    7f60521

    src: fix TLSWrap lifetime bug in ALPN callback

    483a037

  27. magnetnation commented on Apr 25, 2024

    @magnetnation
    magnetnation
    on Apr 25, 2024

    We are using v20.11.0. The error is still present.

  28. ivosh commented on May 17, 2024

    @ivosh
    ivosh
    on May 17, 2024

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    tlsIssues and PRs related to the tls subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions