Skip to content

crypto,quic: add NULL checks for OpenSSL allocation functions#63040

Open
armorbreak001 wants to merge 1 commit intonodejs:mainfrom
armorbreak001:fix/crypto-null-checks
Open

crypto,quic: add NULL checks for OpenSSL allocation functions#63040
armorbreak001 wants to merge 1 commit intonodejs:mainfrom
armorbreak001:fix/crypto-null-checks

Conversation

@armorbreak001
Copy link
Copy Markdown

@armorbreak001 armorbreak001 commented Apr 29, 2026

Fixes: #62774

This PR adds graceful NULL checks for OpenSSL allocation function return values, replacing CHECK() assertions that would abort the process on allocation failure.

Changes

src/crypto/crypto_aes.ccAES_Cipher()

Replaced CHECK(ctx) with:

if (!ctx) {
  return WebCryptoCipherStatus::FAILED;
}

This matches the pattern already used in AES_CTR_Cipher2() in the same file.

src/crypto/crypto_cipher.ccCipherBase::CommonInit()

Replaced CHECK(ctx_) with:

if (!ctx_) {
  return ThrowCryptoError(env(),
                          mark_pop_error_on_return.peekError(),
                          "Failed to allocate cipher context");
}

This matches the error handling pattern used elsewhere in the same function.

Note on other locations from #62774

  • AES_CTR_Cipher2() — Already has a proper if (!ctx) check (line 238)
  • TLSSession::Initialize() — Already has a proper if (!ssl) check (line 794)
  • ECKeyExportTraits::DoExport() — This function no longer exists in the current codebase; the EC key handling code has been refactored

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. labels Apr 29, 2026
Replace CHECK() assertions with graceful error handling for
EVP_CIPHER_CTX_new() allocation failures.

- src/crypto/crypto_aes.cc: In AES_Cipher(), replace CHECK(ctx)
  with early return of WebCryptoCipherStatus::FAILED, matching
  the pattern already used in AES_CTR_Cipher2() in the same file.

- src/crypto/crypto_cipher.cc: In CipherBase::CommonInit(),
  replace CHECK(ctx_) with ThrowCryptoError(), matching the error
  handling pattern used elsewhere in the function.

Note: The other locations mentioned in nodejs#62774 (AES_CTR_Cipher2,
TLSSession::Initialize, and ECKeyExportTraits::DoExport) already
have proper NULL checks in the current codebase or have been
refactored such that the relevant code no longer exists.

Fixes: nodejs#62774
@armorbreak001 armorbreak001 force-pushed the fix/crypto-null-checks branch from 234ce29 to 78ab09a Compare April 29, 2026 20:59
@armorbreak001 armorbreak001 changed the title crypto: add NULL checks for OpenSSL allocation functions crypto,quic: add NULL checks for OpenSSL allocation functions Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. 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