Skip to content

Commit

Permalink
crypto: fix assertion caused by unsupported ext
Browse files Browse the repository at this point in the history
`X509V3_EXT_print` can return value different from `1` if the X509
extension does not support printing to a buffer. Instead of failing with
an unrecoverable assertion - replace the relevant value in the hashmap
with a JS null value.

Fixes: https://hackerone.com/reports/746733

PR-URL: nodejs-private/node-private#175
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
  • Loading branch information
indutny authored and BethGriggs committed Feb 3, 2020
1 parent 25b6897 commit 1156a9e
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 2 deletions.
7 changes: 5 additions & 2 deletions src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2011,8 +2011,11 @@ static Local<Object> X509ToObject(Environment* env, X509* cert) {
X509_EXTENSION* ext = X509_get_ext(cert, index);
CHECK_NOT_NULL(ext);

if (!SafeX509ExtPrint(bio.get(), ext)) {
CHECK_EQ(1, X509V3_EXT_print(bio.get(), ext, 0, 0));
if (!SafeX509ExtPrint(bio.get(), ext) &&
X509V3_EXT_print(bio.get(), ext, 0, 0) != 1) {
info->Set(context, keys[i], Null(env->isolate())).Check();
USE(BIO_reset(bio.get()));
continue;
}

BIO_get_mem_ptr(bio.get(), &mem);
Expand Down
79 changes: 79 additions & 0 deletions test/parallel/test-tls-cert-ext-encoding.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
'use strict';
const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

// NOTE: This certificate is hand-generated, hence it is not located in
// `test/fixtures/keys` to avoid confusion.
//
// The key property of this cert is that subjectAltName contains a string with
// a type `23` which cannot be encoded into string by `X509V3_EXT_print`.
const pem = `
-----BEGIN RSA PRIVATE KEY-----
MIIEowIBAAKCAQEAzrmfPz5M3wTq2/CwMeSQr/N+R1FCJ+O5n+SMleKvBqaK63eJ
kL4BnySMc+ZLKCt4UQSsPFIBK63QFq8n6/vjuTDMJiBTsvzytw8zJt1Zr2HA71N3
VIPt6NdJ/w5lgddTYxR7XudJZJ5lk3PkG8ZgrhuenPYP80UJYVzAC2YZ9KYe3r2B
rVbut1j+8h0TwVcx2Zg5PorsC/EVxHwo4dCmIHceodikr3UVqHneRcrDBytdG6Mo
IqHhZJwBeii/EES9tpWwWbzYYh+38aGGLIF2h5UlVpr0bdBVVUg+uVX3y/Qmu2Qv
4CrAO2IPV6JER9Niwl3ktzNjOMAUQG6BCRSqRQIDAQABAoIBAAmB0+cOsG5ZRYvT
5+aDgnv1EMuq2wYGnRTTZ/vErxP5OM5XcwYrFtwAzEzQPIieZywisOEdTFx74+QH
LijWLsTnj5v5RKAorejpVArnhyZfsoXPKt/CKYDZ1ddbDCQKiRU3be0RafisqDM9
0zHLz8pyDrtdPaKMfD/0Cgj8KxlrLTmfD4otPXds8fZpQe1hR1y12XKVp47l1siW
qFGTaUPDJpQ67xybR08x5DOqmyo4cNMOuReRWrc/qRbWint9U1882eOH09gVfpJZ
Gp6FZVPSgz10MZdLSPLhXqZkY4IxIvNltjBDqkmivd12CD+GVr0qUmTJHzTpk+kG
/CWuRQkCgYEA4EFf8SJHEl0fLDJnOQFyUPY3MalMuopUkQ5CBUe3QXjQhHXsRDfj
Ci/lyzShJkHPbMDHb/rx3lYZB0xNhwnMWKS1gCFVgOCOTZLfD0K1Anxc1hOSgVxI
y5FdO9VW7oQNlsMH/WuDHps0HhJW/00lcrmdyoUM1+fE/3yPQndhUmMCgYEA6/z6
8Gq4PHHNql+gwunAH2cZKNdmcP4Co8MvXCZwIJsLenUuLIZQ/YBKZoM/y5X/cFAG
WFJJuUe6KFetPaDm6NgZgpOmawyUwd5czDjJ6wWgsRywiTISInfJlgWLBVMOuba7
iBL9Xuy0hmcbj0ByoRW9l3gCiBX3yJw3I6wqXTcCgYBnjei22eRF15iIeTHvQfq+
5iNwnEQhM7V/Uj0sYQR/iEGJmUaj7ca6somDf2cW2nblOlQeIpxD1jAyjYqTW/Pv
zwc9BqeMHqW3rqWwT1Z0smbQODOD5tB6qEKMWaSN+Y6o2qC65kWjAXpclI110PME
+i+iEDRxEsaGT8d7otLfDwKBgQCs+xBaQG/x5p2SAGzP0xYALstzc4jk1FzM+5rw
mkBgtiXQyqpg+sfNOkfPIvAVZEsMYax0+0SNKrWbMsGLRjFchmMUovQ+zccQ4NT2
4b2op8Rlbxk8R9ahK1s5u7Bu47YMjZSjJwBQn4OobVX3SI994njJ2a9JX4j0pQWK
AX5AOwKBgAfOsr8HSHTcxSW4F9gegj+hXsRYbdA+eUkFhEGrYyRJgIlQrk/HbuZC
mKd/bQ5R/vwd1cxgV6A0APzpZtbwdhvP0RWji+WnPPovgGcfK0AHFstHnga67/uu
h2LHnKQZ1qWHn+BXWo5d7hBRwWVaK66g3GDN0blZpSz1kKcpy1Pl
-----END RSA PRIVATE KEY-----
-----BEGIN CERTIFICATE-----
MIICwjCCAaqgAwIBAgIDAQABMA0GCSqGSIb3DQEBDQUAMBUxEzARBgNVBAMWCmxv
Y2FsLmhvc3QwHhcNMTkxMjA1MDQyODMzWhcNNDQxMTI5MDQyODMzWjAVMRMwEQYD
VQQDFgpsb2NhbC5ob3N0MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA
zrmfPz5M3wTq2/CwMeSQr/N+R1FCJ+O5n+SMleKvBqaK63eJkL4BnySMc+ZLKCt4
UQSsPFIBK63QFq8n6/vjuTDMJiBTsvzytw8zJt1Zr2HA71N3VIPt6NdJ/w5lgddT
YxR7XudJZJ5lk3PkG8ZgrhuenPYP80UJYVzAC2YZ9KYe3r2BrVbut1j+8h0TwVcx
2Zg5PorsC/EVxHwo4dCmIHceodikr3UVqHneRcrDBytdG6MoIqHhZJwBeii/EES9
tpWwWbzYYh+38aGGLIF2h5UlVpr0bdBVVUg+uVX3y/Qmu2Qv4CrAO2IPV6JER9Ni
wl3ktzNjOMAUQG6BCRSqRQIDAQABoxswGTAXBgNVHREEEDAOlwwqLmxvY2FsLmhv
c3QwDQYJKoZIhvcNAQENBQADggEBAH5ThRLDLwOGuhKsifyiq7k8gbx1FqRegO7H
SIiIYYB35v5Pk0ZPN8QBJwNQzJEjUMjCpHXNdBxknBXRaA8vkbnryMfJm37gPTwA
m6r0uEG78WgcEAe8bgf9iKtQGP/iydKXpSSpDgKoHbswIxD5qtzT+o6VNnkRTSfK
/OGwakluFSoJ/Q9rLpR8lKjA01BhetXMmHbETiY8LSkxOymMldXSzUTD1WdrVn8U
L3dobxT//R/0GraKXG02mf3gZNlb0MMTvW0pVwVy39YmcPEGh8L0hWh1rpAA/VXC
f79uOowv3lLTzQ9na5EThA0tp8d837hdYrrIHh5cfTqBDxG0Tu8=
-----END CERTIFICATE-----
`;

const tls = require('tls');

const options = {
key: pem,
cert: pem,
};

const server = tls.createServer(options, (socket) => {
socket.end();
});
server.listen(0, common.mustCall(function() {
const client = tls.connect({
port: this.address().port,
rejectUnauthorized: false
}, common.mustCall(() => {
// This should not crash process:
client.getPeerCertificate();

server.close();
client.end();
}));
}));

0 comments on commit 1156a9e

Please sign in to comment.