Skip to content

Commit

Permalink
crypto: fix incorrect use of INT_MAX in validation
Browse files Browse the repository at this point in the history
The native crypto module doesn't export INT_MAX, so all occurrences
in the JavaScript layer evaluated to undefined. This change removes
all such occurrences and replaces validateInt32 with validateUint32
since the native layer assumes uint32_t anyway.

The alternative would be to use the constant from the constants
module, but that would be pointless as far as I can tell.

PR-URL: #22581
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
  • Loading branch information
tniessen authored and targos committed Sep 3, 2018
1 parent c47c79e commit 989fd73
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 19 deletions.
8 changes: 4 additions & 4 deletions lib/internal/crypto/pbkdf2.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

const { AsyncWrap, Providers } = process.binding('async_wrap');
const { Buffer } = require('buffer');
const { INT_MAX, pbkdf2: _pbkdf2 } = process.binding('crypto');
const { validateInt32 } = require('internal/validators');
const { pbkdf2: _pbkdf2 } = process.binding('crypto');
const { validateUint32 } = require('internal/validators');
const {
ERR_CRYPTO_INVALID_DIGEST,
ERR_CRYPTO_PBKDF2_ERROR,
Expand Down Expand Up @@ -59,8 +59,8 @@ function check(password, salt, iterations, keylen, digest, callback) {

password = validateArrayBufferView(password, 'password');
salt = validateArrayBufferView(salt, 'salt');
iterations = validateInt32(iterations, 'iterations', 0, INT_MAX);
keylen = validateInt32(keylen, 'keylen', 0, INT_MAX);
iterations = validateUint32(iterations, 'iterations', 0);
keylen = validateUint32(keylen, 'keylen', 0);

return { password, salt, iterations, keylen, digest };
}
Expand Down
20 changes: 10 additions & 10 deletions lib/internal/crypto/scrypt.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

const { AsyncWrap, Providers } = process.binding('async_wrap');
const { Buffer } = require('buffer');
const { INT_MAX, scrypt: _scrypt } = process.binding('crypto');
const { validateInt32 } = require('internal/validators');
const { scrypt: _scrypt } = process.binding('crypto');
const { validateUint32 } = require('internal/validators');
const {
ERR_CRYPTO_SCRYPT_INVALID_PARAMETER,
ERR_CRYPTO_SCRYPT_NOT_SUPPORTED,
Expand Down Expand Up @@ -76,31 +76,31 @@ function check(password, salt, keylen, options, callback) {

password = validateArrayBufferView(password, 'password');
salt = validateArrayBufferView(salt, 'salt');
keylen = validateInt32(keylen, 'keylen', 0, INT_MAX);
keylen = validateUint32(keylen, 'keylen');

let { N, r, p, maxmem } = defaults;
if (options && options !== defaults) {
let has_N, has_r, has_p;
if (has_N = (options.N !== undefined))
N = validateInt32(options.N, 'N', 0, INT_MAX);
N = validateUint32(options.N, 'N');
if (options.cost !== undefined) {
if (has_N) throw new ERR_CRYPTO_SCRYPT_INVALID_PARAMETER();
N = validateInt32(options.cost, 'cost', 0, INT_MAX);
N = validateUint32(options.cost, 'cost');
}
if (has_r = (options.r !== undefined))
r = validateInt32(options.r, 'r', 0, INT_MAX);
r = validateUint32(options.r, 'r');
if (options.blockSize !== undefined) {
if (has_r) throw new ERR_CRYPTO_SCRYPT_INVALID_PARAMETER();
r = validateInt32(options.blockSize, 'blockSize', 0, INT_MAX);
r = validateUint32(options.blockSize, 'blockSize');
}
if (has_p = (options.p !== undefined))
p = validateInt32(options.p, 'p', 0, INT_MAX);
p = validateUint32(options.p, 'p');
if (options.parallelization !== undefined) {
if (has_p) throw new ERR_CRYPTO_SCRYPT_INVALID_PARAMETER();
p = validateInt32(options.parallelization, 'parallelization', 0, INT_MAX);
p = validateUint32(options.parallelization, 'parallelization');
}
if (options.maxmem !== undefined)
maxmem = validateInt32(options.maxmem, 'maxmem', 0, INT_MAX);
maxmem = validateUint32(options.maxmem, 'maxmem');
if (N === 0) N = defaults.N;
if (r === 0) r = defaults.r;
if (p === 0) p = defaults.p;
Expand Down
8 changes: 3 additions & 5 deletions test/parallel/test-crypto-pbkdf2.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ if (!common.hasCrypto)
const assert = require('assert');
const crypto = require('crypto');

const { INT_MAX } = process.binding('constants').crypto;

//
// Test PBKDF2 with RFC 6070 test vectors (except #4)
//
Expand Down Expand Up @@ -71,7 +69,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 >= 0 && <= 2147483647. Received -1'
'It must be >= 0 && < 4294967296. Received -1'
}
);

Expand Down Expand Up @@ -100,7 +98,7 @@ assert.throws(
});
});

[-1, 4073741824, INT_MAX + 1].forEach((input) => {
[-1, 4294967297].forEach((input) => {
assert.throws(
() => {
crypto.pbkdf2('password', 'salt', 1, input, 'sha256',
Expand All @@ -109,7 +107,7 @@ assert.throws(
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError [ERR_OUT_OF_RANGE]',
message: 'The value of "keylen" is out of range. It ' +
`must be >= 0 && <= 2147483647. Received ${input}`
`must be >= 0 && < 4294967296. Received ${input}`
});
});

Expand Down

0 comments on commit 989fd73

Please sign in to comment.