crypto: reject non-int32 values in DiffieHellman()

lib/internal/crypto/diffiehellman.js

@@ -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 {
@@ -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;
@@ -67,7 +77,9 @@ function DiffieHellman(sizeOrKey, keyEncoding, generator, genEncoding) {
 
   if (!generator)
     generator = DH_GENERATOR;
-  else if (typeof generator !== 'number')
+  else if (typeof generator === 'number')
+    validateInt32(generator, 'generator');
+  else
     generator = toBuf(generator, genEncoding);
 
   this[kHandle] = new _DiffieHellman(sizeOrKey, generator);

src/node_crypto.cc

@@ -5136,6 +5136,14 @@ bool DiffieHellman::Init(int primeLength, int g) {
 
 bool DiffieHellman::Init(const char* p, int p_len, int g) {
   dh_.reset(DH_new());
+  if (p_len <= 0) {
+    BNerr(BN_F_BN_GENERATE_PRIME_EX, BN_R_BITS_TOO_SMALL);
+    return false;
+  }
+  if (g <= 1) {
+    DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
+    return false;
+  }
   BIGNUM* bn_p =
       BN_bin2bn(reinterpret_cast<const unsigned char*>(p), p_len, nullptr);
   BIGNUM* bn_g = BN_new();
@@ -5151,10 +5159,23 @@ bool DiffieHellman::Init(const char* p, int p_len, int g) {
 
 bool DiffieHellman::Init(const char* p, int p_len, const char* g, int g_len) {
   dh_.reset(DH_new());
-  BIGNUM* bn_p =
-      BN_bin2bn(reinterpret_cast<const unsigned char*>(p), p_len, nullptr);
+  if (p_len <= 0) {
+    BNerr(BN_F_BN_GENERATE_PRIME_EX, BN_R_BITS_TOO_SMALL);
+    return false;
+  }
+  if (g_len <= 0) {
+    DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
+    return false;
+  }
   BIGNUM* bn_g =
       BN_bin2bn(reinterpret_cast<const unsigned char*>(g), g_len, nullptr);
+  if (BN_is_zero(bn_g) || BN_is_one(bn_g)) {
+    BN_free(bn_g);
+    DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
+    return false;
+  }
+  BIGNUM* bn_p =
+      BN_bin2bn(reinterpret_cast<const unsigned char*>(p), p_len, nullptr);
   if (!DH_set0_pqg(dh_.get(), bn_p, nullptr, bn_g)) {
     BN_free(bn_p);
     BN_free(bn_g);

test/parallel/test-crypto-dh.js

@@ -20,6 +20,62 @@ 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',
+});
+
+assert.throws(() => crypto.createDiffieHellman('abcdef', 13.37), {
+  code: 'ERR_OUT_OF_RANGE',
+  name: 'RangeError',
+  message: 'The value of "generator" 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/,
+  });
+}
+
+// Through a fluke of history, g=0 defaults to DH_GENERATOR (2).
+{
+  const g = 0;
+  crypto.createDiffieHellman('abcdef', g);
+  crypto.createDiffieHellman('abcdef', 'hex', g);
+}
+
+for (const g of [-1, 1]) {
+  const ex = {
+    code: 'ERR_OSSL_DH_BAD_GENERATOR',
+    name: 'Error',
+    message: /bad generator/,
+  };
+  assert.throws(() => crypto.createDiffieHellman('abcdef', g), ex);
+  assert.throws(() => crypto.createDiffieHellman('abcdef', 'hex', g), ex);
+}
+
+crypto.createDiffieHellman('abcdef', Buffer.from([2]));  // OK
+
+for (const g of [Buffer.from([]),
+                 Buffer.from([0]),
+                 Buffer.from([1])]) {
+  const ex = {
+    code: 'ERR_OSSL_DH_BAD_GENERATOR',
+    name: 'Error',
+    message: /bad generator/,
+  };
+  assert.throws(() => crypto.createDiffieHellman('abcdef', g), ex);
+  assert.throws(() => crypto.createDiffieHellman('abcdef', 'hex', g), ex);
+}
+
 {
   const DiffieHellman = crypto.DiffieHellman;
   const dh = DiffieHellman(p1, 'buffer');