http: fix ASAN use-after-poison in onHandshake after checkServerIdentity reject u9sf0l

[robobun]

What

HTTPContext.onHandshake (and the mirror in ProxyTunnel.onHandshake) touched the HTTPClient after checkServerIdentity() returned false, but that path has already freed it.

Repro

Any node:https request whose CA chain verifies but whose hostname doesn't match the server cert's SAN — e.g. connecting to localhost with a cert for CN=agent1 and ca1 trusted — takes the native checkX509ServerIdentity fast path, which fails and returns false from HTTPClient.checkServerIdentity:

==…==ERROR: AddressSanitizer: use-after-poison on address 0x… thread T12 (HTTP Client)
READ of size 2 at 0x…
    #0 … http.HTTPContext.NewHTTPContext(true).Handler.onHandshake … src/http/HTTPContext.zig:420
    #1 … deps.uws.socket.NewSocketHandler(true).configure….on_handshake … src/deps/uws/socket.zig:955
    #2 … us_internal_trigger_handshake_callback … packages/bun-usockets/src/crypto/openssl.c:351

Surfaced by test-https-agent-additional-options, test-https-agent-session-injection, test-https-agent-session-reuse, test-https-agent-sockets-leak, test-https-client-checkServerIdentity, test-https-client-reject, test-tls-set-secure-context under release-asan.

Cause

HTTPClient.checkServerIdentity returning false means it already called closeAndFail:

closeAndFail → terminateSocket + fail
  fail → unregisterAbortTracker + result_callback.run
    onAsyncHTTPCallback → ThreadlocalAsyncHTTP.deinit()  // bun.destroy of the struct that CONTAINS the HTTPClient

So by the time control returns to onHandshake, client points into freed (poisoned) memory. The next lines did a read-modify-write of the packed client.flags (the 2-byte read ASAN caught), called client.unregisterAbortTracker() again, and re-terminated the already-terminated socket.

Fix

Just return when checkServerIdentity yields false. Everything those lines did is already done inside closeAndFail/fail:

• terminateSocket — closeAndFail already called it
• unregisterAbortTracker — first thing fail() does
• did_have_handshaking_error = true — dead store; the only reads of that flag are … and !reject_unauthorized, and this branch is inside if (reject_unauthorized)

Same change applied to the proxy-tunnel handshake which had the identical pattern.

Test

test/js/node/http/node-https-checkServerIdentity-uaf.test.ts spawns a fixture that fires 8 concurrent https.requests whose CA verifies but hostname doesn't, and asserts they all surface ERR_TLS_CERT_ALTNAME_INVALID and exit 0. Under bun bd without this fix the fixture aborts with the ASAN report above; with it, the error events fire and the process exits cleanly.

Note on the Node tests

The 7 Node tests above no longer crash under ASAN but still time out for an unrelated reason: _http_client.ts doesn't forward checkServerIdentity into fetchOptions.tls, so node:https always uses the native fast path, which (unlike Node's tls.checkServerIdentity) doesn't fall back to Subject CN when no SAN is present. That's a separate compat gap and not addressed here, so those tests aren't checked in.

[robobun]

^{Updated 4:42 AM PT - Apr 28th, 2026}

❌ @robobun, your commit ed32ece has 3 failures in Build #48495 (All Failures):

• test/js/bun/s3/s3-storage-class.test.ts - code 1 on 🍎 14 aarch64
• test/js/bun/test/parallel/test-integration-rspack.ts - pid 1968 segmentation fault at address 0x7ffb6b6fb9c8 on 🪟 11 aarch64
• test/js/bun/http/serve-http3.test.ts - code 1 on 🪟 2019 x64
• test/js/bun/http/serve-http3.test.ts - code 1 on 🪟 2019 x64-baseline
• test/js/bun/http/serve-http3.test.ts - code 1 on 🪟 11 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 29819

That installs a local version of the PR into your bun-29819 executable, so you can run:

bun-29819 --bun

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a32dbc38-df71-4235-b615-02cef3092eed

📥 Commits

Reviewing files that changed from the base of the PR and between ca9e089 and ed32ece.

📒 Files selected for processing (4)

• src/http/HTTPContext.zig
• src/http/ProxyTunnel.zig
• test/js/node/http/node-https-checkServerIdentity-uaf-fixture.ts
• test/js/node/http/node-https-checkServerIdentity-uaf.test.ts

---

Walkthrough

Removes redundant error handling in SSL handshake failure paths for HTTPContext and ProxyTunnel by relying on the existing closeAndFail mechanism within checkServerIdentity. Adds regression tests to verify the refactored code doesn't introduce use-after-free vulnerabilities.

Changes

Cohort / File(s)	Summary
Source Code Refactoring src/http/HTTPContext.zig, src/http/ProxyTunnel.zig	Removed post-check failure handling after checkServerIdentity returns false. Code now returns immediately, relying on closeAndFail within the checkServerIdentity failure path to handle socket termination and client cleanup. Added clarifying comments about use-after-free risk.
Regression Tests test/js/node/http/node-https-checkServerIdentity-uaf-fixture.ts, test/js/node/http/node-https-checkServerIdentity-uaf.test.ts	Introduces new test suite to verify that HTTPS requests rejected by checkServerIdentity() during TLS handshake do not trigger use-after-free errors. Fixture reproduces the failure path by connecting with mismatched hostname certificate; test validates clean execution via spawned process.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately identifies the main fix: resolving an ASAN use-after-poison bug in onHandshake after checkServerIdentity rejection.
Description check	✅ Passed	The description provides comprehensive detail covering what the bug is, how it manifests, root cause analysis, the fix, and test coverage, fully satisfying the template requirements.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Found 3 issues this PR may fix:

1. Crash in onHandshake on Windows baseline #24220 - Crash in onHandshake on Windows baseline — direct match: segfault at HTTPContext.zig in the same onHandshake function this PR fixes
2. Segfault regression in v1.2.3 when using TLS with locally signed certificate #19868 - Segfault regression in v1.2.3 when using TLS with locally signed certificate — segfault during TLS rejection path, consistent with the use-after-free in onHandshake after checkServerIdentity rejects
3. MariaDB/MySQL SSL connections broken in Bun 1.3.11 (works in 1.3.10) #28262 - MariaDB/MySQL SSL connections broken in Bun 1.3.11 — involves custom checkServerIdentity handler, the precise code path where the use-after-free occurs

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #24220
Fixes #19868
Fixes #28262

🤖 Generated with Claude Code

[robobun]

Superseded by #29829, which includes this UAF fix plus the CN-fallback / checkServerIdentity forwarding / requestCert forwarding that the same grouped tests need once the crash is out of the way.