From e888f667f5acae55b4604e101f0570e08da8236a Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 14 Jan 2019 12:08:55 +0100 Subject: [PATCH] tls: do not free cert in `.getCertificate()` The documentation of `SSL_get_certificate` states that it returns an internal pointer that must not be freed by the caller. Therefore, using a smart pointer to take ownership is incorrect. Refs: https://man.openbsd.org/SSL_get_certificate.3 Refs: https://github.com/nodejs/node/pull/24261 Fixes: https://github.com/nodejs-private/security/issues/217 PR-URL: https://github.com/nodejs/node/pull/25490 Reviewed-By: Daniel Bevenius Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Sam Roberts Reviewed-By: Luigi Pinca --- src/node_crypto.cc | 6 +++--- test/parallel/test-tls-pfx-authorizationerror.js | 9 +++++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 920869a2f2b..3ff95484871 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -1962,10 +1962,10 @@ void SSLWrap::GetCertificate( Local result; - X509Pointer cert(SSL_get_certificate(w->ssl_.get())); + X509* cert = SSL_get_certificate(w->ssl_.get()); - if (cert) - result = X509ToObject(env, cert.get()); + if (cert != nullptr) + result = X509ToObject(env, cert); args.GetReturnValue().Set(result); } diff --git a/test/parallel/test-tls-pfx-authorizationerror.js b/test/parallel/test-tls-pfx-authorizationerror.js index 6daf89dff0d..5105a60dacd 100644 --- a/test/parallel/test-tls-pfx-authorizationerror.js +++ b/test/parallel/test-tls-pfx-authorizationerror.js @@ -37,8 +37,13 @@ const server = tls rejectUnauthorized: false }, function() { - assert.strictEqual(client.getCertificate().serialNumber, - 'ECC9B856270DA9A8'); + for (let i = 0; i < 10; ++i) { + // Calling this repeatedly is a regression test that verifies + // that .getCertificate() does not accidentally decrease the + // reference count of the X509* certificate on the native side. + assert.strictEqual(client.getCertificate().serialNumber, + 'ECC9B856270DA9A8'); + } client.end(); server.close(); }