Skip to content

Commit b558e9f

Browse files
RafaelGSSrichardlau
authored andcommittedFeb 14, 2023
crypto: clear OpenSSL error on invalid ca cert
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> CVE-ID: CVE-2023-23919 PR-URL: nodejs-private/node-private#375 Refs: https://hackerone.com/bugs?report_id=1808596 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Richard Lau <rlau@redhat.com>
1 parent 160adb7 commit b558e9f

File tree

2 files changed

+49
-1
lines changed

2 files changed

+49
-1
lines changed
 

‎src/crypto/crypto_x509.cc

+4
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,7 @@ void X509Certificate::Pem(const FunctionCallbackInfo<Value>& args) {
340340

341341
void X509Certificate::CheckCA(const FunctionCallbackInfo<Value>& args) {
342342
X509Certificate* cert;
343+
ClearErrorOnReturn clear_error_on_return;
343344
ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
344345
args.GetReturnValue().Set(X509_check_ca(cert->get()) == 1);
345346
}
@@ -440,6 +441,8 @@ void X509Certificate::CheckIssued(const FunctionCallbackInfo<Value>& args) {
440441
X509Certificate* issuer;
441442
ASSIGN_OR_RETURN_UNWRAP(&issuer, args[0]);
442443

444+
ClearErrorOnReturn clear_error_on_return;
445+
443446
args.GetReturnValue().Set(
444447
X509_check_issued(issuer->get(), cert->get()) == X509_V_OK);
445448
}
@@ -482,6 +485,7 @@ void X509Certificate::ToLegacy(const FunctionCallbackInfo<Value>& args) {
482485
Environment* env = Environment::GetCurrent(args);
483486
X509Certificate* cert;
484487
ASSIGN_OR_RETURN_UNWRAP(&cert, args.Holder());
488+
ClearErrorOnReturn clear_error_on_return;
485489
Local<Value> ret;
486490
if (X509ToObject(env, cert->get(), true).ToLocal(&ret))
487491
args.GetReturnValue().Set(ret);

‎test/parallel/test-crypto-x509.js

+45-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const {
99
X509Certificate,
1010
createPrivateKey,
1111
generateKeyPairSync,
12+
createSign,
1213
} = require('crypto');
1314

1415
const {
@@ -191,14 +192,57 @@ const der = Buffer.from(
191192
{
192193
// https://github.com/nodejs/node/issues/45377
193194
// https://github.com/nodejs/node/issues/45485
194-
// Confirm failures of X509Certificate:verify() and X509Certificate:CheckPrivateKey()
195+
// Confirm failures of
196+
// X509Certificate:verify()
197+
// X509Certificate:CheckPrivateKey()
198+
// X509Certificate:CheckCA()
199+
// X509Certificate:CheckIssued()
200+
// X509Certificate:ToLegacy()
195201
// do not affect other functions that use OpenSSL.
196202
// Subsequent calls to e.g. createPrivateKey should not throw.
197203
const keyPair = generateKeyPairSync('ed25519');
198204
assert(!x509.verify(keyPair.publicKey));
199205
createPrivateKey(key);
200206
assert(!x509.checkPrivateKey(keyPair.privateKey));
201207
createPrivateKey(key);
208+
const certPem = `
209+
-----BEGIN CERTIFICATE-----
210+
MIID6zCCAtOgAwIBAgIUTUREAaNcNL0zPkxAlMX0GJtJ/FcwDQYJKoZIhvcNAQEN
211+
BQAwgYkxCzAJBgNVBAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMREwDwYDVQQH
212+
DAhDYXJsc2JhZDEPMA0GA1UECgwGVmlhc2F0MR0wGwYDVQQLDBRWaWFzYXQgU2Vj
213+
dXJlIE1vYmlsZTEiMCAGA1UEAwwZSGFja2VyT25lIHJlcG9ydCAjMTgwODU5NjAi
214+
GA8yMDIyMTIxNjAwMDAwMFoYDzIwMjMxMjE1MjM1OTU5WjCBiTELMAkGA1UEBhMC
215+
VVMxEzARBgNVBAgMCkNhbGlmb3JuaWExETAPBgNVBAcMCENhcmxzYmFkMQ8wDQYD
216+
VQQKDAZWaWFzYXQxHTAbBgNVBAsMFFZpYXNhdCBTZWN1cmUgTW9iaWxlMSIwIAYD
217+
VQQDDBlIYWNrZXJPbmUgcmVwb3J0ICMxODA4NTk2MIIBIjANBgkqhkiG9w0BAQEF
218+
AAOCAQ8AMIIBCgKCAQEA6I7RBPm4E/9rIrCHV5lfsHI/yYzXtACJmoyP8OMkjbeB
219+
h21oSJJF9FEnbivk6bYaHZIPasa+lSAydRM2rbbmfhF+jQoWYCIbV2ztrbFR70S1
220+
wAuJrlYYm+8u+1HUru5UBZWUr/p1gFtv3QjpA8+43iwE4pXytTBKPXFo1f5iZwGI
221+
D5Bz6DohT7Tyb8cpQ1uMCMCT0EJJ4n8wUrvfBgwBO94O4qlhs9vYgnDKepJDjptc
222+
uSuEpvHALO8+EYkQ7nkM4Xzl/WK1yFtxxE93Jvd1OvViDGVrRVfsq+xYTKknGLX0
223+
QIeoDDnIr0OjlYPd/cqyEgMcFyFxwDSzSc1esxdCpQIDAQABo0UwQzAdBgNVHQ4E
224+
FgQUurygsEKdtQk0T+sjM0gEURdveRUwEgYDVR0TAQH/BAgwBgEB/wIB/zAOBgNV
225+
HQ8BAf8EBAMCB4AwDQYJKoZIhvcNAQENBQADggEBAH7mIIXiQsQ4/QGNNFOQzTgP
226+
/bUbMSZJsY5TPAvS9rF9yQVzs4dJZnQk5kEb/qrDQSe27oP0L0hfFm1wTGy+aKfa
227+
BVGHdRmmvHtDUPLA9URCFShqKuS+GXp+6zt7dyZPRrPmiZaciiCMPHOnx59xSdPm
228+
AZG8cD3fmK2ThC4FAMyvRb0qeobka3s22xTQ2kjwJO5gykTkZ+BR6SzRHQTjYMuT
229+
iry9Bu8Kvbzu3r5n+/bmNz+xRNmEeehgT2qsHjA5b2YBVTr9MdN9Ro3H3saA3upr
230+
oans248kpal88CGqsN2so/wZKxVnpiXlPHMdiNL7hRSUqlHkUi07FrP2Htg8kjI=
231+
-----END CERTIFICATE-----`.trim();
232+
const c = new X509Certificate(certPem);
233+
assert(!c.ca);
234+
const signer = createSign('SHA256');
235+
assert(signer.sign(key, 'hex'));
236+
237+
const c1 = new X509Certificate(certPem);
238+
assert(!c1.checkIssued(c1));
239+
const signer1 = createSign('SHA256');
240+
assert(signer1.sign(key, 'hex'));
241+
242+
const c2 = new X509Certificate(certPem);
243+
assert(c2.toLegacyObject());
244+
const signer2 = createSign('SHA256');
245+
assert(signer2.sign(key, 'hex'));
202246
}
203247

204248
// X509Certificate can be cloned via MessageChannel/MessagePort

0 commit comments

Comments
 (0)
Please sign in to comment.