nodejs / nodejs/node

During OpenSSL callbacks, calling OpenSSL functions is unsafe

Đang mở
#65,035 8 bình luận 0 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

Ngôn ngữ chính
JavaScript
Star
122k
Fork
37.4k
Merge trung bình
4 ngày 2 giờ
Pull request đã merge (30 ngày)
283

Mô tả

Version

v26.5.0

Platform
Darwin rpolzer-macbookpro.roam.internal 25.5.0 Darwin Kernel Version 25.5.0: Tue Jun  9 22:18:58 PDT 2026; root:xnu-12377.121.10~1/RELEASE_ARM64_T6000 arm64
Subsystem

ssl_tls

What steps will reproduce the bug?
  • Calling this._handle.destroySSL() from within the ALPNCallback
  • Calling client._handle.setServername('...') from within the on('keylog') callback
  • Calling this.write('...') from within the ALPNCallback
How often does it reproduce? Is there a required condition?

All of these reproduce every time.

What is the expected behavior? Why is that the expected behavior?

No segfault, no abort, no deadlock - just JS exceptions.

What do you see instead?
  • Calling this._handle.destroySSL() from within the ALPNCallback causes segfault of NodeJS:
* thread #1, name = 'node-MainThread', queue = 'com.apple.main-thread', stop reason = breakpoint 1.1
  * frame #0: 0x0000000188addaf4 libsystem_malloc.dylib`malloc_error_break
    frame #1: 0x0000000188ab19ac libsystem_malloc.dylib`malloc_vreport + 744
    frame #2: 0x0000000188ab528c libsystem_malloc.dylib`malloc_report + 64
    frame #3: 0x0000000188aba648 libsystem_malloc.dylib`___BUG_IN_CLIENT_OF_LIBMALLOC_POINTER_BEING_FREED_WAS_NOT_ALLOCATED + 76
    frame #4: 0x0000000100806e20 libssl.3.dylib`tls_post_process_client_hello + 432
    frame #5: 0x00000001007f6398 libssl.3.dylib`state_machine + 1424
    frame #6: 0x00000001007e369c libssl.3.dylib`ssl3_read_bytes + 1788
    frame #7: 0x00000001007982d4 libssl.3.dylib`ssl3_read_internal + 136
    frame #8: 0x00000001007a34fc libssl.3.dylib`SSL_read + 28
    frame #9: 0x0000000104b017cc libnode.147.dylib`node::crypto::TLSWrap::ClearOut() + 324
    frame #10: 0x0000000104b00c4c libnode.147.dylib`node::crypto::TLSWrap::Cycle() + 52
    frame #11: 0x0000000104b03558 libnode.147.dylib`node::crypto::TLSWrap::OnStreamRead(long, uv_buf_t const&) + 124
    frame #12: 0x0000000104a7d40c libnode.147.dylib`node::LibuvStreamWrap::OnUvRead(long, uv_buf_t const*) + 780
    frame #13: 0x0000000104a7d810 libnode.147.dylib`node::LibuvStreamWrap::ReadStart()::$_1::__invoke(uv_stream_s*, long, uv_buf_t const*) + 88
    frame #14: 0x0000000100080660 libuv.1.dylib`uv__stream_io + 1000
    frame #15: 0x00000001000851b0 libuv.1.dylib`uv__io_poll + 1252
    frame #16: 0x0000000100076b64 libuv.1.dylib`uv_run + 276
    frame #17: 0x0000000104904fd0 libnode.147.dylib`node::SpinEventLoopInternal(node::Environment*) + 252
    frame #18: 0x00000001049dbdd0 libnode.147.dylib`node::NodeMainInstance::Run(node::ExitCode*, node::Environment*) + 184
    frame #19: 0x00000001049dbb04 libnode.147.dylib`node::NodeMainInstance::Run() + 140
    frame #20: 0x000000010497cfac libnode.147.dylib`node::Start(int, char**) + 684
    frame #21: 0x00000001888f3e00 dyld`start + 6992
  • Calling client._handle.setServername('...') from within the on('keylog') callback causes assertion failure of NodeJS:
  #  node[11780]: static void node::crypto::TLSWrap::SetServername(const FunctionCallbackInfo<Value> &) at ../src/crypto/crypto_tls.cc:1381
  #  Assertion failed: !wrap->started_

----- Native stack trace -----

 1: 0x105e96010 node::Assert(node::AssertionInfo const&) [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
 2: 0x105f27aec node::crypto::TLSWrap::SetServername(v8::FunctionCallbackInfo<v8::Value> const&) (.cold.5) [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
 3: 0x104ec407c node::crypto::TLSWrap::SetServername(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
 4: 0x10579720c Builtins_CallApiCallbackGeneric [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
 5: 0x105795424 Builtins_InterpreterEntryTrampoline [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
 6: 0x1175013b0 
 7: 0x105795424 Builtins_InterpreterEntryTrampoline [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
 8: 0x105792788 Builtins_JSEntryTrampoline [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
 9: 0x105792478 Builtins_JSEntry [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
10: 0x105010884 v8::internal::(anonymous namespace)::Invoke(v8::internal::Isolate*, v8::internal::(anonymous namespace)::InvokeParams const&) [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
11: 0x1050102bc v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::DirectHandle<v8::internal::Object>, v8::internal::DirectHandle<v8::internal::Object>, v8::base::Vector<v8::internal::DirectHandle<v8::internal::Object> const>) [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
12: 0x105f2e540 v8::Function::Call(v8::Isolate*, v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
13: 0x104cc4310 node::InternalMakeCallback(node::Environment*, v8::Local<v8::Object>, v8::Local<v8::Object>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*, node::async_context, v8::Local<v8::Value>) [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
14: 0x104cd2e6c node::AsyncWrap::MakeCallback(v8::Local<v8::Function>, int, v8::Local<v8::Value>*) [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
15: 0x105f24cd0 node::crypto::(anonymous namespace)::KeylogCallback(ssl_st const*, char const*) [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
16: 0x100941234 nss_keylog_int [/Users/rpolzer/homebrew/Cellar/openssl@3/3.6.3/lib/libssl.3.dylib]
17: 0x1009533ac tls13_change_cipher_state [/Users/rpolzer/homebrew/Cellar/openssl@3/3.6.3/lib/libssl.3.dylib]
18: 0x1009916cc tls_process_server_hello [/Users/rpolzer/homebrew/Cellar/openssl@3/3.6.3/lib/libssl.3.dylib]
19: 0x10098e468 state_machine [/Users/rpolzer/homebrew/Cellar/openssl@3/3.6.3/lib/libssl.3.dylib]
20: 0x10097b69c ssl3_read_bytes [/Users/rpolzer/homebrew/Cellar/openssl@3/3.6.3/lib/libssl.3.dylib]
21: 0x1009302d4 ssl3_read_internal [/Users/rpolzer/homebrew/Cellar/openssl@3/3.6.3/lib/libssl.3.dylib]
22: 0x10093b4fc SSL_read [/Users/rpolzer/homebrew/Cellar/openssl@3/3.6.3/lib/libssl.3.dylib]
23: 0x104ec17cc node::crypto::TLSWrap::ClearOut() [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
24: 0x104ec0c4c node::crypto::TLSWrap::Cycle() [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
25: 0x104ec3558 node::crypto::TLSWrap::OnStreamRead(long, uv_buf_t const&) [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
26: 0x104e3d40c node::LibuvStreamWrap::OnUvRead(long, uv_buf_t const*) [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
27: 0x104e3d810 node::LibuvStreamWrap::ReadStart()::$_1::__invoke(uv_stream_s*, long, uv_buf_t const*) [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
28: 0x100258660 uv__stream_io [/Users/rpolzer/homebrew/Cellar/libuv/1.52.1/lib/libuv.1.0.0.dylib]
29: 0x10025d1b0 uv__io_poll [/Users/rpolzer/homebrew/Cellar/libuv/1.52.1/lib/libuv.1.0.0.dylib]
30: 0x10024eb64 uv_run [/Users/rpolzer/homebrew/Cellar/libuv/1.52.1/lib/libuv.1.0.0.dylib]
31: 0x104cc4fd0 node::SpinEventLoopInternal(node::Environment*) [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
32: 0x104d9bdd0 node::NodeMainInstance::Run(node::ExitCode*, node::Environment*) [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
33: 0x104d9bb04 node::NodeMainInstance::Run() [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
34: 0x104d3cfac node::Start(int, char**) [/Users/rpolzer/homebrew/Cellar/node/26.5.0/lib/libnode.147.dylib]
35: 0x1888f3e00 start [/usr/lib/dyld]

----- JavaScript stack trace -----

1: /private/tmp/z.js:41:23
2: emit (node:events:509:20)
3: onkeylog (node:internal/tls/wrap:554:22)
  • Calling this.write('...') from within the ALPNCallback causes deadlock (due to SSL_write within SSL_do_handshake), the SSL state machine seems to never finish, so the tls.connect callback never runs.
Additional information

I can provide the exact .js files upon request.

It would help if this._handle were not accessible but hidden within a closure, but it would not be sufficient (as you see, the deadlock one doesn't even need this).

We plan to harden BoringSSL against issues like this by having reentrant or concurrent calls to SSL * functions (and maybe others) return a new ERR_R_BAD_CONCURRENCY which would safely prevent such issues; it would however force NodeJS to move to a different approach of providing these callbacks. In particular, NodeJS's own test suite assumes that you can call write from onhandshakedone, which, albeit convenient, breaks invariants within both OpenSSL and BoringSSL and just "happens to work" for now, but will be broken by the pending change.

You can look at our pending patch for BoringSSL here: https://boringssl-review.googlesource.com/c/boringssl/+/97087 - this issue was actually found by testing this patch against known code bases.

Possible solutions include:

  • Set a flag while in an OpenSSL callback, and have further SSL methods check that flag and throw immediately.
  • Defer callbacks to the next frame where possible (primarily, where the callback's return value is not needed).

See also a related issue in CPython, found by the same audit: https://github.com/python/cpython/issues/143756

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Hướng nghiên cứu

Báo cáo nêu ALPNCallback, KeylogCallback, TLSWrap::ClearOut/Cycle, src/crypto/crypto_tls.cc và internal/tls/wrap; hãy bắt đầu bằng cách lần theo các đường đi của callback đó và tương tác với OpenSSL. Công việc được xem là hoàn tất khi các kịch bản callback được liệt kê không còn gây ra segfault, assert hoặc deadlock, và các lệnh gọi reentrant không an toàn nhận được hành vi an toàn như dự kiến.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
javascript, node.js
Lĩnh vực
backend, security
Loại issue
Lỗi
Độ khó
5/5
Thời gian dự kiến
Hơn một tuần
Mức độ hoạt động
Ít trao đổi
Độ rõ ràng
Khá rõ ràng
Mức phù hợp với người mới
35/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.