Skip to content

Commit 1246c94

Browse files
author
Spike
committed
ae.net.ssl: Fix chain-cert double-free in setIdentityFromPKCS12
SSL_CTX_add_extra_chain_cert steals ownership of the X509* it receives (does not bump refcount). The previous code called OPENSSL_sk_pop_free on the ca stack after the add loop, freeing every element a second time. Use OPENSSL_sk_shift to pop-and-transfer each cert one at a time. On success the stack is empty, so the scope(exit) pop_free is a no-op. On partial failure (add throws), remaining un-transferred certs in the stack are freed correctly by scope(exit).
1 parent b064b56 commit 1246c94

1 file changed

Lines changed: 7 additions & 3 deletions

File tree

net/ssl/openssl.d

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ class OpenSSLContext : SSLContext
309309
import deimos.openssl.evp : EVP_PKEY_free;
310310
import deimos.openssl.x509 : X509_free;
311311
import deimos.openssl.safestack : STACK_OF;
312-
import deimos.openssl.stack : _STACK, OPENSSL_sk_num, OPENSSL_sk_value, OPENSSL_sk_pop_free;
312+
import deimos.openssl.stack : _STACK, OPENSSL_sk_num, OPENSSL_sk_free, OPENSSL_sk_shift, OPENSSL_sk_pop_free;
313313

314314
auto bio = BIO_new_mem_buf(cast(void*) data.ptr, cast(int) data.length)
315315
.sslEnforce("BIO_new_mem_buf");
@@ -328,17 +328,21 @@ class OpenSSLContext : SSLContext
328328
scope(exit) { if (cert) X509_free(cert); }
329329
alias CertFreeFunc = extern(C) void function(void*);
330330
auto caRaw = cast(_STACK*) ca;
331+
// Shift each cert out of the stack before transferring to SSL_CTX.
332+
// SSL_CTX_add_extra_chain_cert steals ownership; on partial failure
333+
// remaining certs are still in caRaw and freed by scope(exit) pop_free.
331334
scope(exit) { if (caRaw) OPENSSL_sk_pop_free(caRaw, cast(CertFreeFunc) &X509_free); }
332335

333336
SSL_CTX_use_certificate(sslCtx, cert).sslEnforce("SSL_CTX_use_certificate");
334337
SSL_CTX_use_PrivateKey(sslCtx, pkey).sslEnforce("SSL_CTX_use_PrivateKey");
335338
if (caRaw)
336339
{
337-
foreach (i; 0 .. OPENSSL_sk_num(caRaw))
340+
while (OPENSSL_sk_num(caRaw) > 0)
338341
{
339-
auto chainCert = cast(X509*) OPENSSL_sk_value(caRaw, i);
342+
auto chainCert = cast(X509*) OPENSSL_sk_shift(caRaw);
340343
SSL_CTX_add_extra_chain_cert(sslCtx, chainCert).sslEnforce("SSL_CTX_add_extra_chain_cert");
341344
}
345+
// caRaw is now empty; pop_free on empty stack is a no-op.
342346
}
343347
} /// ditto
344348

0 commit comments

Comments
 (0)