Skip to content

Commit

Permalink
crypto: key size must be int32 in DiffieHellman()
Browse files Browse the repository at this point in the history
The JS code accepted any value where `typeof sizeOrKey === 'number'`
was true but the C++ code checked that `args[0]->IsInt32()` and
subsequently aborted.

Fixes: #32738

PR-URL: #32739
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
bnoordhuis authored and targos committed May 13, 2020
1 parent e05c29d commit 4236175
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
12 changes: 11 additions & 1 deletion lib/internal/crypto/diffiehellman.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ const {
ERR_INVALID_ARG_TYPE,
ERR_INVALID_OPT_VALUE
} = require('internal/errors').codes;
const { validateString } = require('internal/validators');
const {
validateString,
validateInt32,
} = require('internal/validators');
const { isArrayBufferView } = require('internal/util/types');
const { KeyObject } = require('internal/crypto/keys');
const {
Expand Down Expand Up @@ -51,6 +54,13 @@ function DiffieHellman(sizeOrKey, keyEncoding, generator, genEncoding) {
);
}

// Sizes < 0 don't make sense but they _are_ accepted (and subsequently
// rejected with ERR_OSSL_BN_BITS_TOO_SMALL) by OpenSSL. The glue code
// in node_crypto.cc accepts values that are IsInt32() for that reason
// and that's why we do that here too.
if (typeof sizeOrKey === 'number')
validateInt32(sizeOrKey, 'sizeOrKey');

if (keyEncoding && !Buffer.isEncoding(keyEncoding) &&
keyEncoding !== 'buffer') {
genEncoding = generator;
Expand Down
18 changes: 18 additions & 0 deletions test/parallel/test-crypto-dh.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,24 @@ assert.strictEqual(secret2.toString('base64'), secret1);
assert.strictEqual(dh1.verifyError, 0);
assert.strictEqual(dh2.verifyError, 0);

// https://github.com/nodejs/node/issues/32738
// XXX(bnoordhuis) validateInt32() throwing ERR_OUT_OF_RANGE and RangeError
// instead of ERR_INVALID_ARG_TYPE and TypeError is questionable, IMO.
assert.throws(() => crypto.createDiffieHellman(13.37), {
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError',
message: 'The value of "sizeOrKey" is out of range. ' +
'It must be an integer. Received 13.37',
});

for (const bits of [-1, 0, 1]) {
assert.throws(() => crypto.createDiffieHellman(bits), {
code: 'ERR_OSSL_BN_BITS_TOO_SMALL',
name: 'Error',
message: /bits too small/,
});
}

{
const DiffieHellman = crypto.DiffieHellman;
const dh = DiffieHellman(p1, 'buffer');
Expand Down

0 comments on commit 4236175

Please sign in to comment.