Skip to content

Commit 80ee6c0

Browse files
committed
crypto: check DiffieHellman p and g params
It's possible to pass in the prime and generator params as buffers but that mode of input wasn't as rigorously checked as numeric input.
1 parent a3d413a commit 80ee6c0

File tree

2 files changed

+54
-2
lines changed

2 files changed

+54
-2
lines changed

src/node_crypto.cc

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5136,6 +5136,14 @@ bool DiffieHellman::Init(int primeLength, int g) {
51365136

51375137
bool DiffieHellman::Init(const char* p, int p_len, int g) {
51385138
dh_.reset(DH_new());
5139+
if (p_len <= 0) {
5140+
BNerr(BN_F_BN_GENERATE_PRIME_EX, BN_R_BITS_TOO_SMALL);
5141+
return false;
5142+
}
5143+
if (g <= 1) {
5144+
DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
5145+
return false;
5146+
}
51395147
BIGNUM* bn_p =
51405148
BN_bin2bn(reinterpret_cast<const unsigned char*>(p), p_len, nullptr);
51415149
BIGNUM* bn_g = BN_new();
@@ -5151,10 +5159,23 @@ bool DiffieHellman::Init(const char* p, int p_len, int g) {
51515159

51525160
bool DiffieHellman::Init(const char* p, int p_len, const char* g, int g_len) {
51535161
dh_.reset(DH_new());
5154-
BIGNUM* bn_p =
5155-
BN_bin2bn(reinterpret_cast<const unsigned char*>(p), p_len, nullptr);
5162+
if (p_len <= 0) {
5163+
BNerr(BN_F_BN_GENERATE_PRIME_EX, BN_R_BITS_TOO_SMALL);
5164+
return false;
5165+
}
5166+
if (g_len <= 0) {
5167+
DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
5168+
return false;
5169+
}
51565170
BIGNUM* bn_g =
51575171
BN_bin2bn(reinterpret_cast<const unsigned char*>(g), g_len, nullptr);
5172+
if (BN_is_zero(bn_g) || BN_is_one(bn_g)) {
5173+
BN_free(bn_g);
5174+
DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
5175+
return false;
5176+
}
5177+
BIGNUM* bn_p =
5178+
BN_bin2bn(reinterpret_cast<const unsigned char*>(p), p_len, nullptr);
51585179
if (!DH_set0_pqg(dh_.get(), bn_p, nullptr, bn_g)) {
51595180
BN_free(bn_p);
51605181
BN_free(bn_g);

test/parallel/test-crypto-dh.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,37 @@ for (const bits of [-1, 0, 1]) {
4545
});
4646
}
4747

48+
// Through a fluke of history, g=0 defaults to DH_GENERATOR (2).
49+
{
50+
const g = 0;
51+
crypto.createDiffieHellman('abcdef', g);
52+
crypto.createDiffieHellman('abcdef', 'hex', g);
53+
}
54+
55+
for (const g of [-1, 1]) {
56+
const ex = {
57+
code: 'ERR_OSSL_DH_BAD_GENERATOR',
58+
name: 'Error',
59+
message: /bad generator/,
60+
};
61+
assert.throws(() => crypto.createDiffieHellman('abcdef', g), ex);
62+
assert.throws(() => crypto.createDiffieHellman('abcdef', 'hex', g), ex);
63+
}
64+
65+
crypto.createDiffieHellman('abcdef', Buffer.from([2])); // OK
66+
67+
for (const g of [Buffer.from([]),
68+
Buffer.from([0]),
69+
Buffer.from([1])]) {
70+
const ex = {
71+
code: 'ERR_OSSL_DH_BAD_GENERATOR',
72+
name: 'Error',
73+
message: /bad generator/,
74+
};
75+
assert.throws(() => crypto.createDiffieHellman('abcdef', g), ex);
76+
assert.throws(() => crypto.createDiffieHellman('abcdef', 'hex', g), ex);
77+
}
78+
4879
{
4980
const DiffieHellman = crypto.DiffieHellman;
5081
const dh = DiffieHellman(p1, 'buffer');

0 commit comments

Comments
 (0)