diff options
author | Tobias Nießen <tniessen@tnie.de> | 2019-07-21 21:14:34 +0300 |
---|---|---|
committer | Beth Griggs <Bethany.Griggs@uk.ibm.com> | 2019-10-01 21:20:29 +0300 |
commit | 7735824d2cf88f88f811934834ee28bd575f5326 (patch) | |
tree | 504a661963d248befff674ab4b4ad9f4f30232f1 | |
parent | a3eda2896de71d60e10ec4f3fa0bef0bcdfdaa9b (diff) |
crypto: increase maxmem range from 32 to 53 bits
Fixes: https://github.com/nodejs/node/issues/28755
Backport-PR-URL: https://github.com/nodejs/node/pull/29316
PR-URL: https://github.com/nodejs/node/pull/28799
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
-rw-r--r-- | doc/api/crypto.md | 6 | ||||
-rw-r--r-- | lib/internal/crypto/scrypt.js | 12 | ||||
-rw-r--r-- | src/node_crypto.cc | 7 | ||||
-rw-r--r-- | test/parallel/test-crypto-scrypt.js | 15 |
4 files changed, 32 insertions, 8 deletions
diff --git a/doc/api/crypto.md b/doc/api/crypto.md index b58f4ad0804..c4ce5681b0a 100644 --- a/doc/api/crypto.md +++ b/doc/api/crypto.md @@ -2352,6 +2352,9 @@ request. <!-- YAML added: v10.5.0 changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/28799 + description: The `maxmem` value can now be any safe integer. - version: v10.9.0 pr-url: https://github.com/nodejs/node/pull/21525 description: The `cost`, `blockSize` and `parallelization` option names @@ -2407,6 +2410,9 @@ crypto.scrypt('secret', 'salt', 64, { N: 1024 }, (err, derivedKey) => { <!-- YAML added: v10.5.0 changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/28799 + description: The `maxmem` value can now be any safe integer. - version: v10.9.0 pr-url: https://github.com/nodejs/node/pull/21525 description: The `cost`, `blockSize` and `parallelization` option names diff --git a/lib/internal/crypto/scrypt.js b/lib/internal/crypto/scrypt.js index c35b77203df..88458688719 100644 --- a/lib/internal/crypto/scrypt.js +++ b/lib/internal/crypto/scrypt.js @@ -2,12 +2,12 @@ const { AsyncWrap, Providers } = internalBinding('async_wrap'); const { Buffer } = require('buffer'); -const { scrypt: _scrypt } = process.binding('crypto'); -const { validateUint32 } = require('internal/validators'); +const { scrypt: _scrypt } = internalBinding('crypto'); +const { validateInteger, validateUint32 } = require('internal/validators'); const { ERR_CRYPTO_SCRYPT_INVALID_PARAMETER, ERR_CRYPTO_SCRYPT_NOT_SUPPORTED, - ERR_INVALID_CALLBACK, + ERR_INVALID_CALLBACK } = require('internal/errors').codes; const { getDefaultEncoding, @@ -99,8 +99,10 @@ function check(password, salt, keylen, options) { if (has_p) throw new ERR_CRYPTO_SCRYPT_INVALID_PARAMETER(); p = validateUint32(options.parallelization, 'parallelization'); } - if (options.maxmem !== undefined) - maxmem = validateUint32(options.maxmem, 'maxmem'); + if (options.maxmem !== undefined) { + maxmem = options.maxmem; + validateInteger(maxmem, 'maxmem', 0); + } if (N === 0) N = defaults.N; if (r === 0) r = defaults.r; if (p === 0) p = defaults.p; diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 1d9214f18de..de3e28cfe9c 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -4754,7 +4754,7 @@ struct ScryptJob : public CryptoJob { uint32_t N; uint32_t r; uint32_t p; - uint32_t maxmem; + uint64_t maxmem; CryptoErrorVector errors; inline explicit ScryptJob(Environment* env) : CryptoJob(env) {} @@ -4809,7 +4809,7 @@ void Scrypt(const FunctionCallbackInfo<Value>& args) { CHECK(args[3]->IsUint32()); // N CHECK(args[4]->IsUint32()); // r CHECK(args[5]->IsUint32()); // p - CHECK(args[6]->IsUint32()); // maxmem + CHECK(args[6]->IsNumber()); // maxmem CHECK(args[7]->IsObject() || args[7]->IsUndefined()); // wrap object std::unique_ptr<ScryptJob> job(new ScryptJob(env)); job->keybuf_data = reinterpret_cast<unsigned char*>(Buffer::Data(args[0])); @@ -4819,7 +4819,8 @@ void Scrypt(const FunctionCallbackInfo<Value>& args) { job->N = args[3].As<Uint32>()->Value(); job->r = args[4].As<Uint32>()->Value(); job->p = args[5].As<Uint32>()->Value(); - job->maxmem = args[6].As<Uint32>()->Value(); + Local<Context> ctx = env->isolate()->GetCurrentContext(); + job->maxmem = static_cast<uint64_t>(args[6]->IntegerValue(ctx).ToChecked()); if (!job->Validate()) { // EVP_PBE_scrypt() does not always put errors on the error stack // and therefore ToResult() may or may not return an exception diff --git a/test/parallel/test-crypto-scrypt.js b/test/parallel/test-crypto-scrypt.js index 908179b46e3..848b4b38863 100644 --- a/test/parallel/test-crypto-scrypt.js +++ b/test/parallel/test-crypto-scrypt.js @@ -217,3 +217,18 @@ for (const { args, expected } of badargs) { common.expectsError(() => crypto.scrypt('', '', 42, {}), expected); common.expectsError(() => crypto.scrypt('', '', 42, {}, {}), expected); } + +{ + // Values for maxmem that do not fit in 32 bits but that are still safe + // integers should be allowed. + crypto.scrypt('', '', 4, { maxmem: 2 ** 52 }, + common.mustCall((err, actual) => { + assert.ifError(err); + assert.strictEqual(actual.toString('hex'), 'd72c87d0'); + })); + + // Values that exceed Number.isSafeInteger should not be allowed. + common.expectsError(() => crypto.scryptSync('', '', 0, { maxmem: 2 ** 53 }), { + code: 'ERR_OUT_OF_RANGE' + }); +} |