From 7735824d2cf88f88f811934834ee28bd575f5326 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= <tniessen@tnie.de>
Date: Sun, 21 Jul 2019 20:14:34 +0200
Subject: [PATCH] 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>
---
 doc/api/crypto.md                   |  6 ++++++
 lib/internal/crypto/scrypt.js       | 12 +++++++-----
 src/node_crypto.cc                  |  7 ++++---
 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 b58f4ad0804021..c4ce5681b0a95b 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 c35b77203dfc3d..8845868871996e 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 1d9214f18dee58..de3e28cfe9c539 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 908179b46e3071..848b4b38863c47 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'
+  });
+}