crypto: fix assertion caused by unsupported ext

`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 <vieuxtech@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Shelley Vohr <codebytere@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
BridgeAR · Feb 6, 2020 · 861d3f7 · 861d3f7
1 parent fe39757
commit 861d3f7
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 2 deletions.
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -1995,8 +1995,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);

diff --git a/test/parallel/test-tls-cert-ext-encoding.js b/test/parallel/test-tls-cert-ext-encoding.js
@@ -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();
+  }));
+}));