Skip to content

crypto: add NULL check for SSL_new() result in SSLPointer::New#62819

Open
Jah-yee wants to merge 1 commit intonodejs:mainfrom
Jah-yee:fix/ssl-null-check
Open

crypto: add NULL check for SSL_new() result in SSLPointer::New#62819
Jah-yee wants to merge 1 commit intonodejs:mainfrom
Jah-yee:fix/ssl-null-check

Conversation

@Jah-yee
Copy link
Copy Markdown

@Jah-yee Jah-yee commented Apr 19, 2026

Good day

This PR addresses a potential NULL pointer dereference identified by static analysis (Svace) in the OpenSSL allocation functions.

Problem

In deps/ncrypto/ncrypto.cc, the SSLPointer::New() function calls SSL_new() and passes its result directly to the SSLPointer constructor without checking if the allocation failed:

return SSLPointer(SSL_new(ctx.get()));

If OpenSSL fails to allocate an SSL structure (e.g., under memory pressure), SSL_new() returns NULL, which would then be stored in the SSLPointer and potentially dereferenced later, causing a segmentation fault.

Solution

Added an explicit NULL check after SSL_new(), consistent with the pattern already used by CipherCtxPointer::New() in the same file:

SSL* ssl = SSL_new(ctx.get());
if (ssl == nullptr) return {};
return SSLPointer(ssl);

Testing

The change follows the established pattern in the codebase and does not alter any existing behavior for the success case. Callers of SSLPointer::New() already check the returned value (e.g., if (!ssl)), so they will properly handle the NULL case after this fix.

Related Issue

Fixes #62774

Thank you for your attention. If there are any issues or suggestions, please leave a comment and I will address them promptly.

Warmly,
RoomWithOutRoof

Add explicit NULL check for SSL_new() return value in SSLPointer::New(),
consistent with the pattern used by CipherCtxPointer::New().

Fixes a potential NULL pointer dereference when OpenSSL fails to allocate
an SSL structure under memory pressure (Coverity CID 15826735).

Fixes: nodejs#62774
@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto
  • @nodejs/security-wg

@nodejs-github-bot nodejs-github-bot added dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. labels Apr 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

crypto,quic: missing NULL checks for OpenSSL allocation functions

2 participants