From 0f3b679f783fd79b05c19fce9d4e69ccf2bb6791 Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Thu, 19 Feb 2026 09:32:23 +0000 Subject: [PATCH] fix(tls): guard against null cert in checkServerIdentity call When `getPeerCertificate(true)` returns undefined (e.g. due to SSL handle or cert chain being unavailable), `checkServerIdentity` would be called with `undefined` as the cert argument, causing a TypeError: "Cannot destructure property 'subject' from null or undefined value". Add a null check on the cert before passing it to `checkServerIdentity` in both TLS handshake handlers in net.ts. Closes #23556 Co-Authored-By: Claude --- src/js/node/net.ts | 8 +- test/regression/issue/23556.test.ts | 114 ++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 2 deletions(-) create mode 100644 test/regression/issue/23556.test.ts diff --git a/src/js/node/net.ts b/src/js/node/net.ts index bb339ab35b..a1eb432987 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -253,7 +253,9 @@ const SocketHandlers: SocketHandler = { const { checkServerIdentity } = self[bunTLSConnectOptions]; if (!verifyError && typeof checkServerIdentity === "function" && self.servername) { const cert = self.getPeerCertificate(true); - verifyError = checkServerIdentity(self.servername, cert); + if (cert) { + verifyError = checkServerIdentity(self.servername, cert); + } } if (self._requestCert || self._rejectUnauthorized) { if (verifyError) { @@ -563,7 +565,9 @@ const SocketHandlers2: SocketHandler { + const { promise: serverListening, resolve: resolveListening } = Promise.withResolvers(); + const { promise: done, resolve: resolveDone, reject: rejectDone } = Promise.withResolvers(); + + const server = tls.createServer(COMMON_CERT, socket => { + socket.end(); + }); + + server.listen(0, "127.0.0.1", () => { + resolveListening(); + }); + + await serverListening; + const address = server.address() as { port: number }; + + const socket = tls.connect( + { + port: address.port, + host: "127.0.0.1", + servername: "localhost", + rejectUnauthorized: false, + checkServerIdentity: (hostname: string, cert: any) => { + // Before the fix, cert could be undefined which would crash + // with: "Cannot destructure property 'subject' from null or undefined" + expect(cert).toBeDefined(); + expect(cert).not.toBeNull(); + expect(cert.subject).toBeDefined(); + return undefined; + }, + }, + () => { + socket.end(); + server.close(); + resolveDone(); + }, + ); + + socket.on("error", err => { + server.close(); + rejectDone(err); + }); + + await done; +}); + +test("no crash when getPeerCertificate returns undefined during handshake", async () => { + const { promise: serverListening, resolve: resolveListening } = Promise.withResolvers(); + const { promise: done, resolve: resolveDone, reject: rejectDone } = Promise.withResolvers(); + + const server = tls.createServer(COMMON_CERT, socket => { + socket.end(); + }); + + server.listen(0, "127.0.0.1", () => { + resolveListening(); + }); + + await serverListening; + const address = server.address() as { port: number }; + + let checkCalledWithUndefined = false; + + const socket = tls.connect( + { + port: address.port, + host: "127.0.0.1", + servername: "localhost", + rejectUnauthorized: false, + checkServerIdentity: (hostname: string, cert: any) => { + if (cert === undefined || cert === null) { + checkCalledWithUndefined = true; + } + return undefined; + }, + }, + () => { + // Monkey-patch getPeerCertificate BEFORE the handshake completes wouldn't + // work because the callback fires after handshake. But the guard in net.ts + // ensures checkServerIdentity is never called with undefined cert. + // This test verifies the connection succeeds without errors. + expect(checkCalledWithUndefined).toBe(false); + socket.end(); + server.close(); + resolveDone(); + }, + ); + + socket.on("error", err => { + server.close(); + rejectDone(err); + }); + + await done; +}); + +test("tls.checkServerIdentity with null cert throws TypeError", () => { + // The default checkServerIdentity should throw a clear error when + // called with null/undefined cert, not a cryptic destructuring error. + expect(() => { + tls.checkServerIdentity("example.com", null as any); + }).toThrow(); + + expect(() => { + tls.checkServerIdentity("example.com", undefined as any); + }).toThrow(); +});