Skip to content

Commit e8b70fc

Browse files
wintersteigerwintersteiger
authored
Fix memory leaks in OpenSSL crypto (#2210)
1 parent c1ba762 commit e8b70fc

File tree

4 files changed

+134
-89
lines changed

4 files changed

+134
-89
lines changed

‎.daily_canary‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Flime is a tat circle. Flunchtime toubly so.
1+
Always choose the lesser of two weevils.

‎src/tls/key_pair_openssl.h‎

Lines changed: 121 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,114 @@
1313

1414
namespace tls
1515
{
16-
inline void OPENSSL_CHECK1(int rc)
16+
namespace
1717
{
18-
unsigned long ec = ERR_get_error();
19-
if (rc != 1 && ec != 0)
18+
inline void OPENSSL_CHECK1(int rc)
2019
{
21-
throw std::runtime_error(
22-
fmt::format("OpenSSL error: {}", ERR_error_string(ec, NULL)));
20+
unsigned long ec = ERR_get_error();
21+
if (rc != 1 && ec != 0)
22+
{
23+
throw std::runtime_error(
24+
fmt::format("OpenSSL error: {}", ERR_error_string(ec, NULL)));
25+
}
2326
}
24-
}
2527

26-
inline void OPENSSL_CHECKNULL(void* ptr)
27-
{
28-
if (ptr == NULL)
28+
inline void OPENSSL_CHECKNULL(void* ptr)
2929
{
30-
throw std::runtime_error("OpenSSL error: missing object");
30+
if (ptr == NULL)
31+
{
32+
throw std::runtime_error("OpenSSL error: missing object");
33+
}
3134
}
35+
36+
class Unique_BIO
37+
{
38+
std::unique_ptr<BIO, void (*)(BIO*)> p;
39+
40+
public:
41+
Unique_BIO() : p(BIO_new(BIO_s_mem()), [](auto x) { BIO_free(x); })
42+
{
43+
if (!p)
44+
throw std::runtime_error("out of memory");
45+
}
46+
Unique_BIO(const void* buf, int len) :
47+
p(BIO_new_mem_buf(buf, len), [](auto x) { BIO_free(x); })
48+
{
49+
if (!p)
50+
throw std::runtime_error("out of memory");
51+
}
52+
operator BIO*()
53+
{
54+
return p.get();
55+
}
56+
};
57+
58+
class Unique_EVP_PKEY_CTX
59+
{
60+
std::unique_ptr<EVP_PKEY_CTX, void (*)(EVP_PKEY_CTX*)> p;
61+
62+
public:
63+
Unique_EVP_PKEY_CTX(EVP_PKEY* key) :
64+
p(EVP_PKEY_CTX_new(key, NULL), EVP_PKEY_CTX_free)
65+
{
66+
if (!p)
67+
throw std::runtime_error("out of memory");
68+
}
69+
Unique_EVP_PKEY_CTX() :
70+
p(EVP_PKEY_CTX_new_id(EVP_PKEY_EC, NULL), EVP_PKEY_CTX_free)
71+
{
72+
if (!p)
73+
throw std::runtime_error("out of memory");
74+
}
75+
operator EVP_PKEY_CTX*()
76+
{
77+
return p.get();
78+
}
79+
};
80+
81+
class Unique_X509_REQ
82+
{
83+
std::unique_ptr<X509_REQ, void (*)(X509_REQ*)> p;
84+
85+
public:
86+
Unique_X509_REQ() : p(X509_REQ_new(), X509_REQ_free)
87+
{
88+
if (!p)
89+
throw std::runtime_error("out of memory");
90+
}
91+
Unique_X509_REQ(BIO* mem) :
92+
p(PEM_read_bio_X509_REQ(mem, NULL, NULL, NULL), X509_REQ_free)
93+
{
94+
if (!p)
95+
throw std::runtime_error("out of memory");
96+
}
97+
operator X509_REQ*()
98+
{
99+
return p.get();
100+
}
101+
};
102+
103+
class Unique_X509
104+
{
105+
std::unique_ptr<X509, void (*)(X509*)> p;
106+
107+
public:
108+
Unique_X509() : p(X509_new(), X509_free)
109+
{
110+
if (!p)
111+
throw std::runtime_error("out of memory");
112+
}
113+
Unique_X509(BIO* mem) :
114+
p(PEM_read_bio_X509(mem, NULL, NULL, NULL), X509_free)
115+
{
116+
if (!p)
117+
throw std::runtime_error("out of memory");
118+
}
119+
operator X509*()
120+
{
121+
return p.get();
122+
}
123+
};
32124
}
33125

34126
class PublicKey_OpenSSL : public PublicKeyBase
@@ -66,9 +158,8 @@ namespace tls
66158
*/
67159
PublicKey_OpenSSL(const Pem& pem)
68160
{
69-
BIO* mem = BIO_new_mem_buf(pem.data(), -1);
161+
Unique_BIO mem(pem.data(), -1);
70162
key = PEM_read_bio_PUBKEY(mem, NULL, NULL, NULL);
71-
BIO_free(mem);
72163
if (!key)
73164
throw std::runtime_error("could not parse PEM");
74165
}
@@ -146,16 +237,14 @@ namespace tls
146237
md_type = get_md_for_ec(get_curve_id());
147238
}
148239

149-
EVP_PKEY_CTX* pctx = NULL;
150-
OPENSSL_CHECKNULL(pctx = EVP_PKEY_CTX_new(key, NULL));
240+
Unique_EVP_PKEY_CTX pctx(key);
151241
OPENSSL_CHECK1(EVP_PKEY_verify_init(pctx));
152242
if (md_type != MDType::NONE)
153243
{
154244
OPENSSL_CHECK1(
155245
EVP_PKEY_CTX_set_signature_md(pctx, get_md_type(md_type)));
156246
}
157247
int rc = EVP_PKEY_verify(pctx, sig, sig_size, hash, hash_size);
158-
EVP_PKEY_CTX_free(pctx);
159248

160249
bool ok = rc == 1;
161250
if (!ok)
@@ -174,24 +263,14 @@ namespace tls
174263
*/
175264
virtual Pem public_key_pem() const override
176265
{
177-
BIO* buf = BIO_new(BIO_s_mem());
178-
if (!buf)
179-
throw std::runtime_error("out of memory");
266+
Unique_BIO buf;
180267

181268
OPENSSL_CHECK1(PEM_write_bio_PUBKEY(buf, key));
182269

183270
BUF_MEM* bptr;
184271
BIO_get_mem_ptr(buf, &bptr);
185-
Pem result((uint8_t*)bptr->data, bptr->length);
186-
BIO_free(buf);
187-
188-
return result;
272+
return Pem((uint8_t*)bptr->data, bptr->length);
189273
}
190-
191-
// EVP_PKEY* get_raw_context() const
192-
// {
193-
// return key;
194-
// }
195274
};
196275

197276
class KeyPair_OpenSSL : public PublicKey_OpenSSL, public KeyPairBase
@@ -241,24 +320,22 @@ namespace tls
241320
{
242321
int curve_nid = get_openssl_group_id(curve_id);
243322
key = EVP_PKEY_new();
244-
EVP_PKEY_CTX* pkctx = EVP_PKEY_CTX_new_id(EVP_PKEY_EC, NULL);
323+
Unique_EVP_PKEY_CTX pkctx;
245324
if (
246325
EVP_PKEY_paramgen_init(pkctx) < 0 ||
247326
EVP_PKEY_CTX_set_ec_paramgen_curve_nid(pkctx, curve_nid) < 0 ||
248327
EVP_PKEY_CTX_set_ec_param_enc(pkctx, OPENSSL_EC_NAMED_CURVE) < 0)
249328
throw std::runtime_error("could not initialize PK context");
250329
if (EVP_PKEY_keygen_init(pkctx) < 0 || EVP_PKEY_keygen(pkctx, &key) < 0)
251330
throw std::runtime_error("could not generate new EC key");
252-
EVP_PKEY_CTX_free(pkctx);
253331
}
254332

255333
KeyPair_OpenSSL(const KeyPair_OpenSSL&) = delete;
256334

257335
KeyPair_OpenSSL(const Pem& pem, CBuffer pw = nullb)
258336
{
259-
BIO* mem = BIO_new_mem_buf(pem.data(), -1);
337+
Unique_BIO mem(pem.data(), -1);
260338
key = PEM_read_bio_PrivateKey(mem, NULL, NULL, (void*)pw.p);
261-
BIO_free(mem);
262339
if (!key)
263340
throw std::runtime_error("could not parse PEM");
264341
}
@@ -279,19 +356,14 @@ namespace tls
279356
*/
280357
virtual Pem private_key_pem() const override
281358
{
282-
BIO* buf = BIO_new(BIO_s_mem());
283-
if (!buf)
284-
throw std::runtime_error("out of memory");
359+
Unique_BIO buf;
285360

286361
OPENSSL_CHECK1(
287362
PEM_write_bio_PrivateKey(buf, key, NULL, NULL, 0, NULL, NULL));
288363

289364
BUF_MEM* bptr;
290365
BIO_get_mem_ptr(buf, &bptr);
291-
Pem result((uint8_t*)bptr->data, bptr->length);
292-
BIO_free(buf);
293-
294-
return result;
366+
return Pem((uint8_t*)bptr->data, bptr->length);
295367
}
296368

297369
/**
@@ -373,11 +445,9 @@ namespace tls
373445
size_t* sig_size,
374446
uint8_t* sig) const override
375447
{
376-
EVP_PKEY_CTX* pctx = NULL;
377-
OPENSSL_CHECKNULL(pctx = EVP_PKEY_CTX_new(key, NULL));
448+
Unique_EVP_PKEY_CTX pctx(key);
378449
OPENSSL_CHECK1(EVP_PKEY_sign_init(pctx));
379450
OPENSSL_CHECK1(EVP_PKEY_sign(pctx, sig, sig_size, hash, hash_size));
380-
EVP_PKEY_CTX_free(pctx);
381451
return 0;
382452
}
383453

@@ -388,12 +458,7 @@ namespace tls
388458
*/
389459
virtual Pem create_csr(const std::string& name) const override
390460
{
391-
X509_REQ* req = NULL;
392-
393-
if (!(req = X509_REQ_new()))
394-
{
395-
throw std::runtime_error("failed to create X509_REQ object");
396-
}
461+
Unique_X509_REQ req;
397462

398463
OPENSSL_CHECK1(X509_REQ_set_pubkey(req, key));
399464

@@ -418,15 +483,12 @@ namespace tls
418483
if (key)
419484
OPENSSL_CHECK1(X509_REQ_sign(req, key, EVP_sha512()));
420485

421-
BIO* mem = BIO_new(BIO_s_mem());
486+
Unique_BIO mem;
422487
OPENSSL_CHECK1(PEM_write_bio_X509_REQ(mem, req));
423488

424489
BUF_MEM* bptr;
425490
BIO_get_mem_ptr(mem, &bptr);
426491
Pem result((uint8_t*)bptr->data, bptr->length);
427-
BIO_free(mem);
428-
429-
X509_REQ_free(req);
430492

431493
return result;
432494
}
@@ -438,14 +500,9 @@ namespace tls
438500
bool ca = false) const override
439501
{
440502
X509* icrt = NULL;
441-
X509_REQ* csr = NULL;
442-
443-
BIO* mem = BIO_new_mem_buf(signing_request.data(), -1);
444-
OPENSSL_CHECKNULL(csr = PEM_read_bio_X509_REQ(mem, NULL, NULL, NULL));
445-
BIO_free(mem);
446-
447-
X509* crt = NULL;
448-
OPENSSL_CHECKNULL(crt = X509_new());
503+
Unique_BIO mem(signing_request.data(), -1);
504+
Unique_X509_REQ csr(mem);
505+
Unique_X509 crt;
449506

450507
OPENSSL_CHECK1(X509_set_version(crt, 2));
451508

@@ -464,9 +521,8 @@ namespace tls
464521
// Add issuer name
465522
if (!issuer_cert.empty())
466523
{
467-
mem = BIO_new_mem_buf(issuer_cert.data(), -1);
468-
OPENSSL_CHECKNULL(icrt = PEM_read_bio_X509(mem, NULL, NULL, NULL));
469-
BIO_free(mem);
524+
Unique_BIO imem(issuer_cert.data(), -1);
525+
OPENSSL_CHECKNULL(icrt = PEM_read_bio_X509(imem, NULL, NULL, NULL));
470526
OPENSSL_CHECK1(X509_set_issuer_name(crt, X509_get_subject_name(icrt)));
471527
}
472528
else
@@ -556,17 +612,13 @@ namespace tls
556612
if (size <= 0)
557613
throw std::runtime_error("could not sign CRT");
558614

559-
mem = BIO_new(BIO_s_mem());
560-
OPENSSL_CHECK1(PEM_write_bio_X509(mem, crt));
615+
Unique_BIO omem;
616+
OPENSSL_CHECK1(PEM_write_bio_X509(omem, crt));
561617

562618
// Export
563619
BUF_MEM* bptr;
564-
BIO_get_mem_ptr(mem, &bptr);
620+
BIO_get_mem_ptr(omem, &bptr);
565621
Pem result((uint8_t*)bptr->data, bptr->length);
566-
BIO_free(mem);
567-
568-
X509_REQ_free(csr);
569-
X509_free(crt);
570622

571623
if (icrt)
572624
X509_free(icrt);

‎src/tls/rsa_key_pair.h‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,8 @@ namespace tls
139139
*/
140140
RSAPublicKey_OpenSSL(const Pem& pem)
141141
{
142-
BIO* mem = BIO_new_mem_buf(pem.data(), -1);
142+
Unique_BIO mem(pem.data(), -1);
143143
key = PEM_read_bio_PUBKEY(mem, NULL, NULL, NULL);
144-
BIO_free(mem);
145144
if (!key || !EVP_PKEY_get0_RSA(key))
146145
{
147146
throw std::logic_error("invalid RSA key");
@@ -187,7 +186,7 @@ namespace tls
187186
const uint8_t* label = nullptr,
188187
size_t label_size = 0)
189188
{
190-
EVP_PKEY_CTX* ctx = EVP_PKEY_CTX_new(key, NULL);
189+
Unique_EVP_PKEY_CTX ctx(key);
191190
OPENSSL_CHECK1(EVP_PKEY_encrypt_init(ctx));
192191
EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_OAEP_PADDING);
193192
EVP_PKEY_CTX_set_rsa_oaep_md(ctx, EVP_sha256());
@@ -375,9 +374,8 @@ namespace tls
375374

376375
RSAKeyPair_OpenSSL(const Pem& pem, CBuffer pw = nullb)
377376
{
378-
BIO* mem = BIO_new_mem_buf(pem.data(), -1);
377+
Unique_BIO mem(pem.data(), -1);
379378
key = PEM_read_bio_PrivateKey(mem, NULL, NULL, (void*)pw.p);
380-
BIO_free(mem);
381379
if (!key)
382380
{
383381
throw std::runtime_error("could not parse PEM");
@@ -404,7 +402,7 @@ namespace tls
404402
label_size = label->size();
405403
}
406404

407-
EVP_PKEY_CTX* ctx = EVP_PKEY_CTX_new(key, NULL);
405+
Unique_EVP_PKEY_CTX ctx(key);
408406
OPENSSL_CHECK1(EVP_PKEY_decrypt_init(ctx));
409407
EVP_PKEY_CTX_set_rsa_padding(ctx, RSA_PKCS1_OAEP_PADDING);
410408
EVP_PKEY_CTX_set_rsa_oaep_md(ctx, EVP_sha256());
@@ -418,7 +416,9 @@ namespace tls
418416
EVP_PKEY_CTX_set0_rsa_oaep_label(ctx, openssl_label, label_size);
419417
}
420418
else
419+
{
421420
EVP_PKEY_CTX_set0_rsa_oaep_label(ctx, NULL, 0);
421+
}
422422

423423
size_t olen;
424424
OPENSSL_CHECK1(
@@ -445,7 +445,7 @@ namespace tls
445445
size_t public_key_size = RSAKeyPair::default_public_key_size,
446446
size_t public_exponent = RSAKeyPair::default_public_exponent)
447447
{
448-
return RSAKeyPairPtr(new RSAKeyPair(public_key_size, public_exponent));
448+
return std::make_shared<RSAKeyPair>(public_key_size, public_exponent);
449449
}
450450

451451
/**

0 commit comments

Comments
 (0)