Skip to content

Conversation

tniessen
Copy link
Member

When an empty string was passed, malloc might have returned a nullptr depending on the platform, causing an assertion failure. This change makes private key parsing behave as public key parsing does, causing a BIO error instead that can be caught in JS.

I am pre-emptively labeling this security due to the simplicity of causing this failure. This currently only affects the most recent release, v11.6.0, and no other release should include #24234 without this fix.

Fixes: #25247

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

When an empty string was passed, malloc might have returned a nullptr
depending on the platform, causing an assertion failure. This change
makes private key parsing behave as public key parsing does, causing
a BIO error instead that can be caught in JS.

Fixes: nodejs#25247
@tniessen tniessen added crypto Issues and PRs related to the crypto subsystem. security Issues and PRs related to security. labels Dec 28, 2018
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Dec 28, 2018
@tniessen
Copy link
Member Author

tniessen commented Dec 29, 2018

@addaleax
Copy link
Member

Landed in fe5b8dc

@addaleax addaleax closed this Dec 30, 2018
pull bot pushed a commit to SimenB/node that referenced this pull request Dec 30, 2018
When an empty string was passed, malloc might have returned a nullptr
depending on the platform, causing an assertion failure. This change
makes private key parsing behave as public key parsing does, causing
a BIO error instead that can be caught in JS.

Fixes: nodejs#25247

PR-URL: nodejs#25248
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Jan 1, 2019
When an empty string was passed, malloc might have returned a nullptr
depending on the platform, causing an assertion failure. This change
makes private key parsing behave as public key parsing does, causing
a BIO error instead that can be caught in JS.

Fixes: #25247

PR-URL: #25248
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
When an empty string was passed, malloc might have returned a nullptr
depending on the platform, causing an assertion failure. This change
makes private key parsing behave as public key parsing does, causing
a BIO error instead that can be caught in JS.

Fixes: nodejs#25247

PR-URL: nodejs#25248
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@BridgeAR BridgeAR mentioned this pull request Jan 16, 2019
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2019
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. security Issues and PRs related to security.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Zero-byte allocation causes assertion failure
6 participants