Skip to content

Commit 9063e84

Browse files
codebyterecodebytere
authored
fix: crash in crypto.createDiffieHellman (electron#27674)
1 parent 87064e5 commit 9063e84

File tree

2 files changed

+42
-24
lines changed

2 files changed

+42
-24
lines changed

‎patches/node/fix_comment_out_incompatible_crypto_modules.patch‎

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,42 +9,46 @@ with what's exposed through BoringSSL. I plan to upstream parts of this or
99
otherwise introduce shims to reduce friction.
1010

1111
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
12-
index 91cb94d8dbe9db0adbee5e005649188e1ccbcbf9..2429ebf6035de392c6bac18c0b924b995fb3daeb 100644
12+
index 91cb94d8dbe9db0adbee5e005649188e1ccbcbf9..c3d12dc4cc18888815ff5e2c30a21974322d1faf 100644
1313
--- a/src/node_crypto.cc
1414
+++ b/src/node_crypto.cc
15-
@@ -5191,6 +5191,7 @@ bool DiffieHellman::Init(int primeLength, int g) {
16-
15+
@@ -5192,11 +5192,11 @@ bool DiffieHellman::Init(int primeLength, int g) {
1716
bool DiffieHellman::Init(const char* p, int p_len, int g) {
1817
dh_.reset(DH_new());
19-
+#if 0
2018
if (p_len <= 0) {
21-
BNerr(BN_F_BN_GENERATE_PRIME_EX, BN_R_BITS_TOO_SMALL);
19+
- BNerr(BN_F_BN_GENERATE_PRIME_EX, BN_R_BITS_TOO_SMALL);
20+
+ OPENSSL_PUT_ERROR(BN, BN_R_BITS_TOO_SMALL);
2221
return false;
23-
@@ -5199,6 +5200,7 @@ bool DiffieHellman::Init(const char* p, int p_len, int g) {
24-
DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
22+
}
23+
if (g <= 1) {
24+
- DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
25+
+ OPENSSL_PUT_ERROR(DH, DH_R_BAD_GENERATOR);
2526
return false;
2627
}
27-
+#endif
2828
BIGNUM* bn_p =
29-
BN_bin2bn(reinterpret_cast<const unsigned char*>(p), p_len, nullptr);
30-
BIGNUM* bn_g = BN_new();
31-
@@ -5214,6 +5216,7 @@ bool DiffieHellman::Init(const char* p, int p_len, int g) {
32-
29+
@@ -5215,18 +5215,18 @@ bool DiffieHellman::Init(const char* p, int p_len, int g) {
3330
bool DiffieHellman::Init(const char* p, int p_len, const char* g, int g_len) {
3431
dh_.reset(DH_new());
35-
+#if 0
3632
if (p_len <= 0) {
37-
BNerr(BN_F_BN_GENERATE_PRIME_EX, BN_R_BITS_TOO_SMALL);
33+
- BNerr(BN_F_BN_GENERATE_PRIME_EX, BN_R_BITS_TOO_SMALL);
34+
+ OPENSSL_PUT_ERROR(BN, BN_R_BITS_TOO_SMALL);
3835
return false;
39-
@@ -5236,6 +5239,7 @@ bool DiffieHellman::Init(const char* p, int p_len, const char* g, int g_len) {
36+
}
37+
if (g_len <= 0) {
38+
- DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
39+
+ OPENSSL_PUT_ERROR(DH, DH_R_BAD_GENERATOR);
40+
return false;
41+
}
42+
BIGNUM* bn_g =
43+
BN_bin2bn(reinterpret_cast<const unsigned char*>(g), g_len, nullptr);
44+
if (BN_is_zero(bn_g) || BN_is_one(bn_g)) {
4045
BN_free(bn_g);
46+
- DHerr(DH_F_DH_BUILTIN_GENPARAMS, DH_R_BAD_GENERATOR);
47+
+ OPENSSL_PUT_ERROR(DH, DH_R_BAD_GENERATOR);
4148
return false;
4249
}
43-
+#endif
44-
return VerifyContext();
45-
}
46-
47-
@@ -5718,8 +5722,9 @@ void ECDH::SetPrivateKey(const FunctionCallbackInfo<Value>& args) {
50+
BIGNUM* bn_p =
51+
@@ -5718,8 +5718,9 @@ void ECDH::SetPrivateKey(const FunctionCallbackInfo<Value>& args) {
4852

4953
if (!EC_KEY_set_public_key(new_key.get(), pub.get()))
5054
return env->ThrowError("Failed to set generated public key");
@@ -55,23 +59,23 @@ index 91cb94d8dbe9db0adbee5e005649188e1ccbcbf9..2429ebf6035de392c6bac18c0b924b99
5559
ecdh->group_ = EC_KEY_get0_group(ecdh->key_.get());
5660
}
5761

58-
@@ -6207,6 +6212,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
62+
@@ -6207,6 +6208,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
5963
EVPKeyCtxPointer Setup() override {
6064
EVPKeyPointer params;
6165
if (prime_info_.fixed_value_) {
6266
+#if 0
6367
DHPointer dh(DH_new());
6468
if (!dh)
6569
return nullptr;
66-
@@ -6223,6 +6229,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
70+
@@ -6223,6 +6225,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
6771
params = EVPKeyPointer(EVP_PKEY_new());
6872
CHECK(params);
6973
EVP_PKEY_assign_DH(params.get(), dh.release());
7074
+#endif
7175
} else {
7276
EVPKeyCtxPointer param_ctx(EVP_PKEY_CTX_new_id(EVP_PKEY_DH, nullptr));
7377
if (!param_ctx)
74-
@@ -6230,7 +6237,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
78+
@@ -6230,7 +6233,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
7579

7680
if (EVP_PKEY_paramgen_init(param_ctx.get()) <= 0)
7781
return nullptr;
@@ -80,7 +84,7 @@ index 91cb94d8dbe9db0adbee5e005649188e1ccbcbf9..2429ebf6035de392c6bac18c0b924b99
8084
if (EVP_PKEY_CTX_set_dh_paramgen_prime_len(param_ctx.get(),
8185
prime_info_.prime_size_) <= 0)
8286
return nullptr;
83-
@@ -6238,7 +6245,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
87+
@@ -6238,7 +6241,7 @@ class DHKeyPairGenerationConfig : public KeyPairGenerationConfig {
8488
if (EVP_PKEY_CTX_set_dh_paramgen_generator(param_ctx.get(),
8589
generator_) <= 0)
8690
return nullptr;

‎spec/node-spec.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,20 @@ describe('node feature', () => {
303303
expect(cipherText).to.equal(result);
304304
}
305305
});
306+
307+
it('does not crash when using crypto.diffieHellman() constructors', () => {
308+
const crypto = require('crypto');
309+
310+
crypto.createDiffieHellman('abc');
311+
crypto.createDiffieHellman('abc', 2);
312+
313+
// Needed to test specific DiffieHellman ctors.
314+
315+
// eslint-disable-next-line no-octal
316+
crypto.createDiffieHellman('abc', Buffer.from([02]));
317+
// eslint-disable-next-line no-octal
318+
crypto.createDiffieHellman('abc', '123');
319+
});
306320
});
307321

308322
describe('process.stdout', () => {

0 commit comments

Comments
 (0)