lib: replace checkUint() with validateInt32()

PR-URL: #20816 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
nodejs · Jun 13, 2018 · f3570f2 · f3570f2
1 parent 02adb2d
commit f3570f2
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 31 deletions.
diff --git a/lib/internal/crypto/pbkdf2.js b/lib/internal/crypto/pbkdf2.js
@@ -2,7 +2,8 @@
 
 const { AsyncWrap, Providers } = process.binding('async_wrap');
 const { Buffer } = require('buffer');
-const { pbkdf2: _pbkdf2 } = process.binding('crypto');
+const { INT_MAX, pbkdf2: _pbkdf2 } = process.binding('crypto');
+const { validateInt32 } = require('internal/validators');
 const {
   ERR_CRYPTO_INVALID_DIGEST,
   ERR_CRYPTO_PBKDF2_ERROR,
@@ -11,7 +12,6 @@ const {
 } = require('internal/errors').codes;
 const {
   checkIsArrayBufferView,
-  checkIsUint,
   getDefaultEncoding,
 } = require('internal/crypto/util');
 
@@ -59,10 +59,8 @@ function check(password, salt, iterations, keylen, digest, callback) {
 
   password = checkIsArrayBufferView('password', password);
   salt = checkIsArrayBufferView('salt', salt);
-  // FIXME(bnoordhuis) The error message is in fact wrong since |iterations|
-  // cannot be > INT_MAX.  Adjust in the next major release.
-  iterations = checkIsUint('iterations', iterations, 'a non-negative number');
-  keylen = checkIsUint('keylen', keylen);
+  iterations = validateInt32(iterations, 'iterations', 0, INT_MAX);
+  keylen = validateInt32(keylen, 'keylen', 0, INT_MAX);
 
   return { password, salt, iterations, keylen, digest };
 }

diff --git a/lib/internal/crypto/scrypt.js b/lib/internal/crypto/scrypt.js
@@ -2,15 +2,15 @@
 
 const { AsyncWrap, Providers } = process.binding('async_wrap');
 const { Buffer } = require('buffer');
-const { scrypt: _scrypt } = process.binding('crypto');
+const { INT_MAX, scrypt: _scrypt } = process.binding('crypto');
+const { validateInt32 } = require('internal/validators');
 const {
   ERR_CRYPTO_SCRYPT_INVALID_PARAMETER,
   ERR_CRYPTO_SCRYPT_NOT_SUPPORTED,
   ERR_INVALID_CALLBACK,
 } = require('internal/errors').codes;
 const {
   checkIsArrayBufferView,
-  checkIsUint,
   getDefaultEncoding,
 } = require('internal/crypto/util');
 
@@ -75,16 +75,19 @@ function check(password, salt, keylen, options, callback) {
     throw new ERR_CRYPTO_SCRYPT_NOT_SUPPORTED();
 
   password = checkIsArrayBufferView('password', password);
-  salt = checkIsArrayBufferView('salt', salt);
-  keylen = checkIsUint('keylen', keylen);
+  salt = checkIsArrayBufferView(salt, 'salt');
+  keylen = validateInt32(keylen, 'keylen', 0, INT_MAX);
 
   let { N, r, p, maxmem } = defaults;
   if (options && options !== defaults) {
-    if (options.hasOwnProperty('N')) N = checkIsUint('N', options.N);
-    if (options.hasOwnProperty('r')) r = checkIsUint('r', options.r);
-    if (options.hasOwnProperty('p')) p = checkIsUint('p', options.p);
+    if (options.hasOwnProperty('N'))
+      N = validateInt32(options.N, 'N', 0, INT_MAX);
+    if (options.hasOwnProperty('r'))
+      r = validateInt32(options.r, 'r', 0, INT_MAX);
+    if (options.hasOwnProperty('p'))
+      p = validateInt32(options.p, 'p', 0, INT_MAX);
     if (options.hasOwnProperty('maxmem'))
-      maxmem = checkIsUint('maxmem', options.maxmem);
+      maxmem = validateInt32(options.maxmem, 'maxmem', 0, INT_MAX);
     if (N === 0) N = defaults.N;
     if (r === 0) r = defaults.r;
     if (p === 0) p = defaults.p;

diff --git a/lib/internal/crypto/util.js b/lib/internal/crypto/util.js
@@ -16,7 +16,6 @@ const {
   ERR_CRYPTO_ENGINE_UNKNOWN,
   ERR_CRYPTO_TIMING_SAFE_EQUAL_LENGTH,
   ERR_INVALID_ARG_TYPE,
-  ERR_OUT_OF_RANGE,
 } = require('internal/errors').codes;
 const { Buffer } = require('buffer');
 const {
@@ -26,9 +25,6 @@ const {
 const {
   isArrayBufferView
 } = require('internal/util/types');
-const {
-  INT_MAX
-} = process.binding('constants').crypto;
 
 var defaultEncoding = 'buffer';
 
@@ -99,19 +95,8 @@ function checkIsArrayBufferView(name, buffer) {
   return buffer;
 }
 
-function checkIsUint(name, value, errmsg = `>= 0 && <= ${INT_MAX}`) {
-  if (typeof value !== 'number')
-    throw new ERR_INVALID_ARG_TYPE(name, 'number', value);
-
-  if (value < 0 || !Number.isInteger(value) || value > INT_MAX)
-    throw new ERR_OUT_OF_RANGE(name, errmsg, value);
-
-  return value;
-}
-
 module.exports = {
   checkIsArrayBufferView,
-  checkIsUint,
   getCiphers,
   getCurves,
   getDefaultEncoding,

diff --git a/lib/internal/validators.js b/lib/internal/validators.js
@@ -71,6 +71,8 @@ function validateInteger(value, name) {
     Error.captureStackTrace(err, validateInteger);
     throw err;
   }
+
+  return value;
 }
 
 function validateInt32(value, name, min = -2147483648, max = 2147483647) {
@@ -91,6 +93,8 @@ function validateInt32(value, name, min = -2147483648, max = 2147483647) {
     Error.captureStackTrace(err, validateInt32);
     throw err;
   }
+
+  return value;
 }
 
 function validateUint32(value, name, positive) {
@@ -112,6 +116,8 @@ function validateUint32(value, name, positive) {
     Error.captureStackTrace(err, validateUint32);
     throw err;
   }
+
+  return value;
 }
 
 module.exports = {

diff --git a/test/parallel/test-crypto-pbkdf2.js b/test/parallel/test-crypto-pbkdf2.js
@@ -71,7 +71,7 @@ assert.throws(
     code: 'ERR_OUT_OF_RANGE',
     name: 'RangeError [ERR_OUT_OF_RANGE]',
     message: 'The value of "iterations" is out of range. ' +
-             'It must be a non-negative number. Received -1'
+             'It must be >= 0 && <= 2147483647. Received -1'
   }
 );
 
@@ -87,7 +87,20 @@ assert.throws(
     });
 });
 
-[Infinity, -Infinity, NaN, -1, 4073741824, INT_MAX + 1].forEach((input) => {
+[Infinity, -Infinity, NaN].forEach((input) => {
+  assert.throws(
+    () => {
+      crypto.pbkdf2('password', 'salt', 1, input, 'sha256',
+                    common.mustNotCall());
+    }, {
+      code: 'ERR_OUT_OF_RANGE',
+      name: 'RangeError [ERR_OUT_OF_RANGE]',
+      message: 'The value of "keylen" is out of range. It ' +
+               `must be an integer. Received ${input}`
+    });
+});
+
+[-1, 4073741824, INT_MAX + 1].forEach((input) => {
   assert.throws(
     () => {
       crypto.pbkdf2('password', 'salt', 1, input, 'sha256',