Skip to content

crypto: Fix potential null pointer dereference when BIO_meth_new() fails#61788

Open
ndossche wants to merge 2 commits intonodejs:mainfrom
ndossche:clesss-1
Open

crypto: Fix potential null pointer dereference when BIO_meth_new() fails#61788
ndossche wants to merge 2 commits intonodejs:mainfrom
ndossche:clesss-1

Conversation

@ndossche
Copy link

@ndossche ndossche commented Feb 12, 2026

This function can return null, which will make the calls to BIO_meth_set_* trigger a null deref.
Even after fixing this, there is an issue with the BIOPointer::New(GetMethod()) call in NodeBIO::New because the New method cannot handle a null pointer despite other code already guarding for this
(e.g. the NodeBIO::New function already checks bio). This patch solves the issues by adding more null checks.

Note: this was found by a static-dynamic analyser I'm developing.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

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

@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. dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. labels Feb 12, 2026
This function can return null, which will make the calls to
BIO_meth_set_* trigger a null deref.
Even after fixing this, there is an issue with the
`BIOPointer::New(GetMethod())` call in `NodeBIO::New` because the
`New` method cannot handle a null pointer despite other code already
guarding for this
(e.g. the `NodeBIO::New` function already checks `bio`).
This patch solves the issues by adding more null checks.
ndossche added a commit to ndossche/ncrypto that referenced this pull request Feb 12, 2026
This function calls BIO_new() which mustn't receive a null pointer
argument. Yet it is able to handle null BIOs gracefully.
To solve this, add a null check.

Ref: nodejs/node#61788
ndossche added a commit to ndossche/ncrypto that referenced this pull request Feb 12, 2026
This function calls BIO_new() which mustn't receive a null pointer
argument. Yet it is able to handle null BIOs gracefully.
To solve this, add a null check.

Ref: nodejs/node#61788
@codecov
Copy link

codecov bot commented Feb 12, 2026

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.73%. Comparing base (4a13a62) to head (6ffa012).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/crypto/crypto_bio.cc 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #61788      +/-   ##
==========================================
- Coverage   89.76%   89.73%   -0.03%     
==========================================
  Files         675      675              
  Lines      204674   204675       +1     
  Branches    39330    39340      +10     
==========================================
- Hits       183716   183656      -60     
- Misses      13235    13293      +58     
- Partials     7723     7726       +3     
Files with missing lines Coverage Δ
src/crypto/crypto_bio.cc 75.52% <0.00%> (-0.27%) ⬇️

... and 35 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@addaleax addaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Feb 13, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 13, 2026
@nodejs-github-bot
Copy link
Collaborator

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

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. 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.

5 participants