From 252358a0a56a8f239100c283ff61ca36ac7d643d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yusufhan=20Sa=C3=A7ak?= Date: Sat, 9 May 2026 15:17:42 +0300 Subject: [PATCH 1/2] fix(node:crypto): align scrypt error code with Node.js The native scrypt path threw a generic Error('Scrypt failed') so node:crypto.scrypt(...) and scryptSync(...) returned an error without the code: 'ERR_CRYPTO_INVALID_SCRYPT_PARAMS' Node.js sets for invalid params. Wrap the native call so the failure surfaces as a RangeError with that code and the 'Invalid scrypt params' message. BoringSSL's EVP_PBE_scrypt also does not enforce 128 * r * (N + p + 2) <= maxmem the way OpenSSL does, so cases like {N:32768, r:8, p:1, maxmem:0} (coalesced to the 32 MiB default) succeeded in workerd while throwing on Node. Add the missing memory check on the JS side, run alongside the native call so the error path matches Node's async (callback) and sync (throw) shapes. Uses BigInt because the operands can each be up to 2**32 - 1, and their product would overflow a JS Number's safe integer range. Tests now assert err.code on bad and toobig vectors, and a regression case mirroring the original issue is added to toobig. Fixes #6639 --- src/node/internal/crypto_scrypt.ts | 39 ++++++++++++++++--- src/node/internal/internal_errors.ts | 6 +++ .../api/node/tests/crypto_scrypt-test.js | 13 +++++-- 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/src/node/internal/crypto_scrypt.ts b/src/node/internal/crypto_scrypt.ts index fc4992c1239..b6096f8dd9f 100644 --- a/src/node/internal/crypto_scrypt.ts +++ b/src/node/internal/crypto_scrypt.ts @@ -39,7 +39,10 @@ import { kMaxLength } from 'node-internal:internal_buffer'; import { getArrayBufferOrView } from 'node-internal:crypto_util'; -import { ERR_INVALID_ARG_VALUE } from 'node-internal:internal_errors'; +import { + ERR_CRYPTO_INVALID_SCRYPT_PARAMS, + ERR_INVALID_ARG_VALUE, +} from 'node-internal:internal_errors'; export interface ValidatedScryptOptions { password: ArrayLike; @@ -145,6 +148,24 @@ function validateParameters( }; } +// OpenSSL's EVP_PBE_scrypt enforces 128 * r * (N + p + 2) <= maxmem; BoringSSL +// does not, so mirror the check here to match Node.js behaviour. Run alongside +// the native call (sync inside scryptSync, inside the Promise body for scrypt) +// so the failure surfaces the same way Node's C++ ScryptJob errors do, rather +// than as a synchronous throw from the validator. Uses BigInt because +// validateUint32 allows values up to 2**32 - 1, whose product would overflow a +// JS Number's safe integer range. +function checkScryptMemory( + N: number, + r: number, + p: number, + maxmem: number +): void { + if (128n * BigInt(r) * (BigInt(N) + BigInt(p) + 2n) > BigInt(maxmem)) { + throw new ERR_CRYPTO_INVALID_SCRYPT_PARAMS(); + } +} + type Callback = (err: Error | null, derivedKey?: Buffer) => void; type OptionsOrCallback = ScryptOptions | Callback; @@ -175,9 +196,10 @@ export function scrypt( new Promise((res, rej) => { try { + checkScryptMemory(N, r, p, maxmem); res(cryptoImpl.getScrypt(password, salt, N, r, p, maxmem, keylen)); - } catch (err) { - rej(err as Error); + } catch { + rej(new ERR_CRYPTO_INVALID_SCRYPT_PARAMS()); } }).then( (val: ArrayBuffer) => { @@ -206,7 +228,12 @@ export function scryptSync( options )); - return Buffer.from( - cryptoImpl.getScrypt(password, salt, N, r, p, maxmem, keylen) - ); + try { + checkScryptMemory(N, r, p, maxmem); + return Buffer.from( + cryptoImpl.getScrypt(password, salt, N, r, p, maxmem, keylen) + ); + } catch { + throw new ERR_CRYPTO_INVALID_SCRYPT_PARAMS(); + } } diff --git a/src/node/internal/internal_errors.ts b/src/node/internal/internal_errors.ts index 554a8d228e7..610dbf10d42 100644 --- a/src/node/internal/internal_errors.ts +++ b/src/node/internal/internal_errors.ts @@ -248,6 +248,12 @@ export class ERR_CRYPTO_INVALID_KEY_OBJECT_TYPE extends NodeError { } } +export class ERR_CRYPTO_INVALID_SCRYPT_PARAMS extends NodeRangeError { + constructor() { + super('ERR_CRYPTO_INVALID_SCRYPT_PARAMS', 'Invalid scrypt params'); + } +} + export class ERR_INVALID_ARG_TYPE_RANGE extends NodeRangeError { constructor(name: string, expected: string | string[], actual: unknown) { const msg = createInvalidArgType(name, expected); diff --git a/src/workerd/api/node/tests/crypto_scrypt-test.js b/src/workerd/api/node/tests/crypto_scrypt-test.js index 58e3c2ac0f6..0543d508c7f 100644 --- a/src/workerd/api/node/tests/crypto_scrypt-test.js +++ b/src/workerd/api/node/tests/crypto_scrypt-test.js @@ -125,6 +125,7 @@ const toobig = [ { N: 2, p: 2 ** 30, r: 1 }, // p > (2**30-1)/r { N: 2 ** 20, p: 1, r: 8 }, { N: 2 ** 10, p: 1, r: 8, maxmem: 2 ** 20 }, + { N: 32768, p: 1, r: 8, maxmem: 0 }, // maxmem coalesces to default; ratio still fails ]; const badargs = [ @@ -200,8 +201,10 @@ export const badTests = { if (err) reject(err); }); scrypt('pass', 'salt', 1, options, fn); - await rejects(promise); - throws(() => scryptSync('pass', 'salt', 1, options)); + await rejects(promise, { code: 'ERR_CRYPTO_INVALID_SCRYPT_PARAMS' }); + throws(() => scryptSync('pass', 'salt', 1, options), { + code: 'ERR_CRYPTO_INVALID_SCRYPT_PARAMS', + }); } throws(() => scryptSync('pass', 'salt', 1, { N: 1, cost: 1 })); throws(() => scryptSync('pass', 'salt', 1, { p: 1, parallelization: 1 })); @@ -217,10 +220,12 @@ export const tooBigTests = { if (err) reject(err); }); scrypt('pass', 'salt', 1, options, fn); - await rejects(promise); + await rejects(promise, { code: 'ERR_CRYPTO_INVALID_SCRYPT_PARAMS' }); strictEqual(fn.mock.calls.length, 1); - throws(() => scryptSync('pass', 'salt', 1, options)); + throws(() => scryptSync('pass', 'salt', 1, options), { + code: 'ERR_CRYPTO_INVALID_SCRYPT_PARAMS', + }); } }, }; From 3eeb4f9c51d73e4588492f11d3937da7a699e127 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yusufhan=20Sa=C3=A7ak?= Date: Sat, 9 May 2026 17:27:26 +0300 Subject: [PATCH 2/2] fix(node:crypto): preserve coded errors through the scrypt rewrap Address review feedback: the previous catch threw a fresh ERR_CRYPTO_INVALID_SCRYPT_PARAMS for anything, which discarded the already-coded error from checkScryptMemory and would have masked unrelated failures from the native call. normaliseScryptError now only rewrites the exact generic Error ('Scrypt failed', no .code) that the workerd C++ scrypt path emits. Anything that already carries a .code (such as the ERR_CRYPTO_INVALID_SCRYPT_PARAMS thrown by the JS-side memory check) or anything with a different message passes through unchanged. --- src/node/internal/crypto_scrypt.ts | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/node/internal/crypto_scrypt.ts b/src/node/internal/crypto_scrypt.ts index b6096f8dd9f..089f33e6b7c 100644 --- a/src/node/internal/crypto_scrypt.ts +++ b/src/node/internal/crypto_scrypt.ts @@ -166,6 +166,23 @@ function checkScryptMemory( } } +// The native cryptoImpl.getScrypt throws a generic Error('Scrypt failed') with +// no .code on invalid scrypt params; rewrap that exact error to carry Node's +// standard ERR_CRYPTO_INVALID_SCRYPT_PARAMS code. Any other error (including +// errors that already have a code, or errors with a different message) is +// returned unchanged so unrelated failures and the already-coded error from +// checkScryptMemory aren't suppressed. +function normaliseScryptError(err: unknown): Error { + if ( + err instanceof Error && + err.message === 'Scrypt failed' && + (err as { code?: unknown }).code === undefined + ) { + return new ERR_CRYPTO_INVALID_SCRYPT_PARAMS(); + } + return err as Error; +} + type Callback = (err: Error | null, derivedKey?: Buffer) => void; type OptionsOrCallback = ScryptOptions | Callback; @@ -198,8 +215,8 @@ export function scrypt( try { checkScryptMemory(N, r, p, maxmem); res(cryptoImpl.getScrypt(password, salt, N, r, p, maxmem, keylen)); - } catch { - rej(new ERR_CRYPTO_INVALID_SCRYPT_PARAMS()); + } catch (err) { + rej(normaliseScryptError(err)); } }).then( (val: ArrayBuffer) => { @@ -233,7 +250,7 @@ export function scryptSync( return Buffer.from( cryptoImpl.getScrypt(password, salt, N, r, p, maxmem, keylen) ); - } catch { - throw new ERR_CRYPTO_INVALID_SCRYPT_PARAMS(); + } catch (err) { + throw normaliseScryptError(err); } }