crypto: coerce -0 keylen to +0 in pbkdf2 and scrypt by ljharb · Pull Request #63531 · nodejs/node

ljharb · 2026-05-24T08:43:03Z

validateInt32(keylen, 'keylen', 0) lets -0 through: typeof -0 is 'number', Number.isInteger(-0) is true, and -0 < 0 is false.
The value then reaches the PBKDF2Job binding, whose IsInt32() check fails (V8 boxes -0 as a HeapNumber rather than a tagged SMI) and aborts the process with SIGABRT.
Coerce keylen to +0 after validation so the binding sees a true Int32.

Reachable from any caller that forwards a JSON-parsed value, since JSON.parse('{"keylen":-0}').keylen preserves the sign.

(an alternative implementation would be to throw on -0 - this seemed friendlier, but I'm happy to do that instead, if folks prefer)

`validateInt32(keylen, 'keylen', 0)` lets `-0` through: `typeof -0` is `'number'`, `Number.isInteger(-0)` is `true`, and `-0 < 0` is `false`. The value then reaches the PBKDF2Job binding, whose `IsInt32()` check fails (V8 boxes `-0` as a HeapNumber rather than a tagged SMI) and aborts the process with SIGABRT. Coerce `keylen` to `+0` after validation so the binding sees a true Int32. Reachable from any caller that forwards a JSON-parsed value, since `JSON.parse('{"keylen":-0}').keylen` preserves the sign. Signed-off-by: Jordan Harband <ljharb@gmail.com>

nodejs-github-bot · 2026-05-24T08:43:09Z

Review requested:

@nodejs/crypto

panva · 2026-05-24T09:11:30Z

@ljharb can you throw in a fix for scrypt too?

panva · 2026-05-24T09:22:10Z

Outside of pbkdf2 and scrypt there are also instances in cipher, diffiehellman, keygen, hash, random. I can tackle those separately in a followup if you want, or you can, up to you.

codecov · 2026-05-24T10:18:11Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.31%. Comparing base (df09b2a) to head (c095afb).
⚠️ Report is 30 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #63531      +/-   ##
==========================================
- Coverage   90.32%   90.31%   -0.02%     
==========================================
  Files         730      730              
  Lines      234152   234156       +4     
  Branches    43900    43912      +12     
==========================================
- Hits       211499   211467      -32     
- Misses      14374    14418      +44     
+ Partials     8279     8271       -8

Files with missing lines	Coverage Δ
lib/internal/crypto/pbkdf2.js	`100.00% <100.00%> (+1.52%)`	⬆️
lib/internal/crypto/scrypt.js	`94.89% <100.00%> (+0.07%)`	⬆️

... and 31 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.

Mirror of the prior pbkdf2 fix. `validateInt32(keylen, 'keylen', 0)` lets `-0` through (since `-0 < 0` is `false`), and the ScryptJob binding's `IsInt32()` check at `crypto_scrypt.cc` aborts the process with SIGABRT because V8 boxes `-0` as a HeapNumber rather than a tagged SMI. Coerce `keylen` to `+0` after validation.

nodejs-github-bot · 2026-05-24T20:58:13Z

CI: https://ci.nodejs.org/job/node-test-pull-request/73682/

akshatgit · 2026-05-25T23:44:59Z

@ljharb — the fix covers pbkdf2/scrypt but the same crash exists in five other APIs, verified on v22.22.1:

node -e "require('crypto').hkdf('sha256','k','s','',-0,()=>{})"          # exit 134, crypto_hkdf.cc:54
node -e "require('crypto').hkdfSync('sha256','k','s','',-0)"              # exit 134, crypto_hkdf.cc:54
node -e "require('crypto').generateKeyPair('rsa',{modulusLength:-0,publicExponent:65537},()=>{})"  # exit 134, crypto_rsa.cc:137
node -e "require('crypto').generateKeyPairSync('rsa',{modulusLength:-0,publicExponent:65537})"     # exit 134, crypto_rsa.cc:137
node -e "require('crypto').createDiffieHellman(-0)"                       # exit 134, crypto_util.h:607

The += 0 coercion from this PR would work in hkdf.js, keygen.js, and dh.js. Or fixing isUint32() in validators.js once covers all callers:

function isUint32(value) {
  return value === (value >>> 0) && !Object.is(value, -0);
}

nodejs-github-bot · 2026-05-26T08:50:08Z

Commit Queue failed

- Loading data for nodejs/node/pull/63531 ✔ Done loading data for nodejs/node/pull/63531 ----------------------------------- PR info ------------------------------------ Title crypto: coerce -0 keylen to +0 in pbkdf2 and scrypt (#63531) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch ljharb:crypto-pbkdf2-negative-zero-keylen -> nodejs:main Labels crypto, author ready, needs-ci, commit-queue-rebase Commits 2 - crypto: coerce -0 keylen to +0 in pbkdf2 - crypto: coerce -0 keylen to +0 in scrypt Committers 1 - Jordan Harband <ljharb@gmail.com> PR-URL: https://github.com/nodejs/node/pull/63531 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/63531 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> -------------------------------------------------------------------------------- ℹ This PR was created on Sun, 24 May 2026 08:43:03 GMT ✔ Approvals: 3 ✔ - Filip Skokan (@panva) (TSC): https://github.com/nodejs/node/pull/63531#pullrequestreview-4352962596 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/63531#pullrequestreview-4353060473 ✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/63531#pullrequestreview-4353331893 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2026-05-24T20:58:13Z: https://ci.nodejs.org/job/node-test-pull-request/73682/ - Querying data for job/node-test-pull-request/73682/ ✔ Build data downloaded ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD ✔ origin/main is now up-to-date - Downloading patch for 63531 From https://github.com/nodejs/node * branch refs/pull/63531/merge -> FETCH_HEAD ✔ Fetched commits as c98ab680d6a0..c095afb5b868 -------------------------------------------------------------------------------- [main a4d78abcbf] crypto: coerce -0 keylen to +0 in pbkdf2 Author: Jordan Harband <ljharb@gmail.com> Date: Sun May 24 10:37:02 2026 +0200 2 files changed, 31 insertions(+) [main 29c93964c6] crypto: coerce -0 keylen to +0 in scrypt Author: Jordan Harband <ljharb@gmail.com> Date: Sun May 24 15:40:32 2026 +0200 2 files changed, 29 insertions(+) ✔ Patches applied There are 2 commits in the PR. Attempting autorebase. (node:352) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated. (Use `node --trace-deprecation ...` to show where the warning was created) Rebasing (2/4)

�[KExecuting: git node land --amend --yes
--------------------------------- New Message ----------------------------------
crypto: coerce -0 keylen to +0 in pbkdf2

validateInt32(keylen, 'keylen', 0) lets -0 through: typeof -0 is
'number', Number.isInteger(-0) is true, and -0 < 0 is false.
The value then reaches the PBKDF2Job binding, whose IsInt32() check
fails (V8 boxes -0 as a HeapNumber rather than a tagged SMI) and
aborts the process with SIGABRT.
Coerce keylen to +0
after validation so the binding sees a true Int32.

Reachable from any caller that forwards a JSON-parsed value,
since JSON.parse('{"keylen":-0}').keylen preserves the sign.

Signed-off-by: Jordan Harband <ljharb@gmail.com>
PR-URL: #63531
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

[detached HEAD 076633221f] crypto: coerce -0 keylen to +0 in pbkdf2
Author: Jordan Harband <ljharb@gmail.com>
Date: Sun May 24 10:37:02 2026 +0200
2 files changed, 31 insertions(+)
Rebasing (3/4)
Rebasing (4/4)

�[KExecuting: git node land --amend --yes
--------------------------------- New Message ----------------------------------
crypto: coerce -0 keylen to +0 in scrypt

Mirror of the prior pbkdf2 fix. validateInt32(keylen, 'keylen', 0)
lets -0 through (since -0 < 0 is false), and the ScryptJob
binding's IsInt32() check at crypto_scrypt.cc aborts the process
with SIGABRT because V8 boxes -0 as a HeapNumber rather than a
tagged SMI. Coerce keylen to +0 after validation.

PR-URL: #63531
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

[detached HEAD 1eabc6dadb] crypto: coerce -0 keylen to +0 in scrypt
Author: Jordan Harband <ljharb@gmail.com>
Date: Sun May 24 15:40:32 2026 +0200
2 files changed, 29 insertions(+)

�[KSuccessfully rebased and updated refs/heads/main.

✔ 076633221f525ae25eca90443b2244898aed7de7
✔ 0:0 no Assisted-by metadata assisted-by-is-trailer
✔ 0:0 no Co-authored-by metadata co-authored-by-is-trailer
✔ 0:0 skipping fixes-url fixes-url
✔ 0:0 blank line after title line-after-title
✔ 0:0 line-lengths are valid line-length
✔ 0:0 metadata is at end of message metadata-end
✔ 13:8 PR-URL is valid. pr-url
✔ 0:0 reviewers are valid reviewers
✔ 0:0 has valid Signed-off-by signed-off-by
✔ 0:0 valid subsystems subsystem
✔ 0:0 Title is formatted correctly. title-format
✔ 0:0 Title is <= 50 columns. title-length
✖ 1eabc6dadb12e85d07a015344231ce0eac3660ef
✔ 0:0 no Assisted-by metadata assisted-by-is-trailer
✔ 0:0 no Co-authored-by metadata co-authored-by-is-trailer
✔ 0:0 skipping fixes-url fixes-url
✔ 0:0 blank line after title line-after-title
✔ 0:0 line-lengths are valid line-length
✔ 0:0 metadata is at end of message metadata-end
✔ 7:8 PR-URL is valid. pr-url
✔ 0:0 reviewers are valid reviewers
✖ 0:0 Commit must have a "Signed-off-by" trailer signed-off-by
✔ 0:0 valid subsystems subsystem
✔ 0:0 Title is formatted correctly. title-format
✔ 0:0 Title is <= 50 columns. title-length

ℹ Please fix the commit message and try again.
Please manually ammend the commit message, by running
git commit --amend
Once commit message is fixed, finish the landing command running
git node land --continue

https://github.com/nodejs/node/actions/runs/26442117825

panva · 2026-05-26T09:27:06Z

Landed in 3393399

`validateInt32(keylen, 'keylen', 0)` lets `-0` through: `typeof -0` is `'number'`, `Number.isInteger(-0)` is `true`, and `-0 < 0` is `false`. The value then reaches the PBKDF2Job binding, whose `IsInt32()` check fails (V8 boxes `-0` as a HeapNumber rather than a tagged SMI) and aborts the process with SIGABRT. Coerce `keylen` to `+0` after validation so the binding sees a true Int32. Reachable from any caller that forwards a JSON-parsed value, since `JSON.parse('{"keylen":-0}').keylen` preserves the sign. Mirror of the prior pbkdf2 fix. `validateInt32(keylen, 'keylen', 0)` lets `-0` through (since `-0 < 0` is `false`), and the ScryptJob binding's `IsInt32()` check at `crypto_scrypt.cc` aborts the process with SIGABRT because V8 boxes `-0` as a HeapNumber rather than a tagged SMI. Coerce `keylen` to `+0` after validation. Signed-off-by: Jordan Harband <ljharb@gmail.com> PR-URL: #63531 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

ljharb · 2026-05-26T15:17:04Z

I'll do a followup for the other locations, especially since the direction is confirmed.

panva · 2026-05-26T15:18:53Z

I'll do a followup for the other locations, especially since the direction is confirmed.

@ljharb #63556

`-0` slips past the `keylen < 0` guard in `precondition` (since `-0 < 0` is `false`) and reaches Node's native pbkdf2 binding, whose `IsInt32()` assertion aborts the process with an uncatchable SIGABRT on Node.js 15+ (V8 holds `-0` as a double, not a tagged Int32). Coerce it to `+0` so the binding sees a true Int32; `-0` now behaves identically to `+0`. Also see nodejs/node#63531

ljharb added the crypto Issues and PRs related to the crypto subsystem. label May 24, 2026

nodejs-github-bot added the needs-ci PRs that need a full CI run. label May 24, 2026

ljharb force-pushed the crypto-pbkdf2-negative-zero-keylen branch 2 times, most recently from e4b488e to 9700ffe Compare May 24, 2026 08:53

This comment was marked as outdated.

Sign in to view

ljharb force-pushed the crypto-pbkdf2-negative-zero-keylen branch from 6a406f2 to c095afb Compare May 24, 2026 13:45

panva added the commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. label May 24, 2026

panva changed the title ~~crypto: coerce -0 keylen to +0 in pbkdf2~~ crypto: coerce -0 keylen to +0 in pbkdf2 and scrypt May 24, 2026

panva approved these changes May 24, 2026

View reviewed changes

panva 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 May 24, 2026

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 24, 2026

This comment was marked as outdated.

Sign in to view

jasnell approved these changes May 24, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

lpinca approved these changes May 24, 2026

View reviewed changes

panva added the commit-queue Add this label to land a pull request using GitHub Actions. label May 25, 2026

panva mentioned this pull request May 25, 2026

lib: coerce -0 to +0 in various APIs #63556

Open

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels May 26, 2026

This comment was marked as resolved.

Sign in to view

panva closed this May 26, 2026

ljharb deleted the crypto-pbkdf2-negative-zero-keylen branch May 26, 2026 15:46

Uh oh!

Conversation

ljharb commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented May 24, 2026

Uh oh!

panva commented May 24, 2026

Uh oh!

panva commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented May 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

nodejs-github-bot commented May 24, 2026

Uh oh!

akshatgit commented May 25, 2026

Uh oh!

nodejs-github-bot commented May 26, 2026

Signed-off-by: Jordan Harband <ljharb@gmail.com> PR-URL: #63531 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

PR-URL: #63531 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

�[KSuccessfully rebased and updated refs/heads/main.

Uh oh!

This comment was marked as resolved.

panva commented May 26, 2026

Uh oh!

ljharb commented May 26, 2026

Uh oh!

panva commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ljharb commented May 24, 2026 •

edited

Loading

panva commented May 24, 2026 •

edited

Loading

codecov Bot commented May 24, 2026 •

edited

Loading

Signed-off-by: Jordan Harband <ljharb@gmail.com>
PR-URL: #63531
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

PR-URL: #63531
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

panva commented May 26, 2026 •

edited

Loading