Skip to content

Commit e0e7763

Browse files
tniessentargos
authored andcommitted
crypto: increase maxmem range from 32 to 53 bits
Fixes: #28755 PR-URL: #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>
1 parent d0b1fb3 commit e0e7763

4 files changed

Lines changed: 28 additions & 6 deletions

File tree

‎doc/api/crypto.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2587,6 +2587,9 @@ request.
25872587
<!-- YAML
25882588
added: v10.5.0
25892589
changes:
2590+
- version: REPLACEME
2591+
pr-url: https://github.com/nodejs/node/pull/28799
2592+
description: The `maxmem` value can now be any safe integer.
25902593
- version: v10.9.0
25912594
pr-url: https://github.com/nodejs/node/pull/21525
25922595
description: The `cost`, `blockSize` and `parallelization` option names
@@ -2641,6 +2644,9 @@ crypto.scrypt('secret', 'salt', 64, { N: 1024 }, (err, derivedKey) => {
26412644
<!-- YAML
26422645
added: v10.5.0
26432646
changes:
2647+
- version: REPLACEME
2648+
pr-url: https://github.com/nodejs/node/pull/28799
2649+
description: The `maxmem` value can now be any safe integer.
26442650
- version: v10.9.0
26452651
pr-url: https://github.com/nodejs/node/pull/21525
26462652
description: The `cost`, `blockSize` and `parallelization` option names

‎lib/internal/crypto/scrypt.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@
33
const{ AsyncWrap, Providers }=internalBinding('async_wrap');
44
const{ Buffer }=require('buffer');
55
const{scrypt: _scrypt}=internalBinding('crypto');
6-
const{ validateUint32 }=require('internal/validators');
6+
const{validateInteger,validateUint32 }=require('internal/validators');
77
const{
88
ERR_CRYPTO_SCRYPT_INVALID_PARAMETER,
99
ERR_CRYPTO_SCRYPT_NOT_SUPPORTED,
10-
ERR_INVALID_CALLBACK,
10+
ERR_INVALID_CALLBACK
1111
}=require('internal/errors').codes;
1212
const{
1313
getDefaultEncoding,
@@ -107,8 +107,8 @@ function check(password, salt, keylen, options) {
107107
p=options.parallelization;
108108
}
109109
if(options.maxmem!==undefined){
110-
validateUint32(options.maxmem,'maxmem');
111110
maxmem=options.maxmem;
111+
validateInteger(maxmem,'maxmem',0);
112112
}
113113
if(N===0)N=defaults.N;
114114
if(r===0)r=defaults.r;

‎src/node_crypto.cc‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6015,7 +6015,7 @@ struct ScryptJob : public CryptoJob {
60156015
uint32_t N;
60166016
uint32_t r;
60176017
uint32_t p;
6018-
uint32_t maxmem;
6018+
uint64_t maxmem;
60196019
CryptoErrorVector errors;
60206020

60216021
inlineexplicitScryptJob(Environment* env) : CryptoJob(env) {}
@@ -6070,7 +6070,7 @@ void Scrypt(const FunctionCallbackInfo<Value>& args) {
60706070
CHECK(args[3]->IsUint32()); // N
60716071
CHECK(args[4]->IsUint32()); // r
60726072
CHECK(args[5]->IsUint32()); // p
6073-
CHECK(args[6]->IsUint32()); // maxmem
6073+
CHECK(args[6]->IsNumber()); // maxmem
60746074
CHECK(args[7]->IsObject() || args[7]->IsUndefined()); // wrap object
60756075
std::unique_ptr<ScryptJob> job(newScryptJob(env));
60766076
job->keybuf_data = reinterpret_cast<unsignedchar*>(Buffer::Data(args[0]));
@@ -6080,7 +6080,8 @@ void Scrypt(const FunctionCallbackInfo<Value>& args) {
60806080
job->N = args[3].As<Uint32>()->Value();
60816081
job->r = args[4].As<Uint32>()->Value();
60826082
job->p = args[5].As<Uint32>()->Value();
6083-
job->maxmem = args[6].As<Uint32>()->Value();
6083+
Local<Context> ctx = env->isolate()->GetCurrentContext();
6084+
job->maxmem = static_cast<uint64_t>(args[6]->IntegerValue(ctx).ToChecked());
60846085
if (!job->Validate()) {
60856086
// EVP_PBE_scrypt() does not always put errors on the error stack
60866087
// and therefore ToResult() may or may not return an exception

‎test/parallel/test-crypto-scrypt.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,3 +220,18 @@ for (const { args, expected } of badargs) {
220220
common.expectsError(()=>crypto.scrypt('','',42,{}),expected);
221221
common.expectsError(()=>crypto.scrypt('','',42,{},{}),expected);
222222
}
223+
224+
{
225+
// Values for maxmem that do not fit in 32 bits but that are still safe
226+
// integers should be allowed.
227+
crypto.scrypt('','',4,{maxmem: 2**52},
228+
common.mustCall((err,actual)=>{
229+
assert.ifError(err);
230+
assert.strictEqual(actual.toString('hex'),'d72c87d0');
231+
}));
232+
233+
// Values that exceed Number.isSafeInteger should not be allowed.
234+
common.expectsError(()=>crypto.scryptSync('','',0,{maxmem: 2**53}),{
235+
code: 'ERR_OUT_OF_RANGE'
236+
});
237+
}

0 commit comments

Comments
 (0)