Skip to content

Commit e888f66

Browse files
committed
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: #24261 Fixes: https://github.com/nodejs-private/security/issues/217 PR-URL: #25490 Reviewed-By: Daniel Bevenius <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
1 parent 9e9890a commit e888f66

File tree

2 files changed

+10
-5
lines changed

2 files changed

+10
-5
lines changed

src/node_crypto.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1962,10 +1962,10 @@ void SSLWrap<Base>::GetCertificate(
19621962

19631963
Local<Object> result;
19641964

1965-
X509Pointer cert(SSL_get_certificate(w->ssl_.get()));
1965+
X509* cert = SSL_get_certificate(w->ssl_.get());
19661966

1967-
if (cert)
1968-
result = X509ToObject(env, cert.get());
1967+
if (cert != nullptr)
1968+
result = X509ToObject(env, cert);
19691969

19701970
args.GetReturnValue().Set(result);
19711971
}

test/parallel/test-tls-pfx-authorizationerror.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,13 @@ const server = tls
3737
rejectUnauthorized: false
3838
},
3939
function() {
40-
assert.strictEqual(client.getCertificate().serialNumber,
41-
'ECC9B856270DA9A8');
40+
for (let i = 0; i < 10; ++i) {
41+
// Calling this repeatedly is a regression test that verifies
42+
// that .getCertificate() does not accidentally decrease the
43+
// reference count of the X509* certificate on the native side.
44+
assert.strictEqual(client.getCertificate().serialNumber,
45+
'ECC9B856270DA9A8');
46+
}
4247
client.end();
4348
server.close();
4449
}

0 commit comments

Comments
 (0)