crypto: coerce -0 keylen to +0 in pbkdf2 and scrypt

[ljharb]

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)

[nodejs-github-bot]

Review requested:

• @nodejs/crypto

[panva]

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

[panva]

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]

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.

[nodejs-github-bot]

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

[akshatgit]

@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]

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 &lt; 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 &lt; 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]

Landed in 3393399

[ljharb]

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

[panva]

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

@ljharb #63556