From ed88e69744eeeac11dc3ace7d61d440fa0c070d0 Mon Sep 17 00:00:00 2001 From: Justas Brazauskas Date: Tue, 7 Jul 2026 10:51:13 +0300 Subject: [PATCH] Fix WorkerEntrypoint use-after-free on subrequest cancellation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `WorkerEntrypoint::requestImpl` is a coroutine whose KJ_DEFER at scope exit runs `incomingRequest->drain(waitUntilTasks, ...)`. The DEFER is invoked either at natural scope exit (with `this` alive) or from the coroutine's destructor. On some subrequest paths the coroutine's destructor fires after the `WorkerEntrypoint` has already been freed by its owning `Own`: the promise chain that owns the coroutine can outlive that Own via the response body's attached task. The trigger observed in practice is a fire-and-forget `env.QUEUE.send()` followed by a 3xx response — the queue producer's loopback subrequest lands its `WorkerEntrypoint::requestImpl` coroutine in `IoContext::tasks`, and when the outer request unwinds that TaskSet the coroutine's DEFER reads `this->waitUntilTasks` from freed memory, gets a null `TaskSet&`, and faults inside `TaskSet::add`. Repro binaries either segfault (bazel test harness) or spin at 100% CPU indefinitely (standalone workerd / miniflare). Bind `waitUntilTasks` to a coroutine-frame local so the DEFER's implicit `[&]` capture pins the reference against the coroutine frame instead of `this`. `WorkerService::waitUntilTasks` itself lives for the service's lifetime, so the underlying `TaskSet` is always valid. The DEFER still reads other `this->*` members (`failOpenService`, `proxyTask`, `loggedExceptionEarlier`, `abortController`, `cfBlobJson`); those are latent UAFs on the same path but haven't been observed to crash — zeroed freed memory happens to steer the DEFER's control flow past them. A proper fix would tie the entrypoint's lifetime to the coroutine's; leaving a `TODO(cleanup)`. Includes a `wd_test` regression that exercises the exact combination (fire-and-forget `env.QUEUE.send()` + `302`) via a self-service binding, so it reproduces inside a single worker with no miniflare. --- src/workerd/api/tests/BUILD.bazel | 6 +++ .../tests/queue-send-redirect-hang-test.js | 47 +++++++++++++++++++ .../queue-send-redirect-hang-test.wd-test | 18 +++++++ src/workerd/io/worker-entrypoint.c++ | 10 ++++ 4 files changed, 81 insertions(+) create mode 100644 src/workerd/api/tests/queue-send-redirect-hang-test.js create mode 100644 src/workerd/api/tests/queue-send-redirect-hang-test.wd-test diff --git a/src/workerd/api/tests/BUILD.bazel b/src/workerd/api/tests/BUILD.bazel index a11facc4e8b..4ced514e608 100644 --- a/src/workerd/api/tests/BUILD.bazel +++ b/src/workerd/api/tests/BUILD.bazel @@ -271,6 +271,12 @@ wd_test( data = ["queue-do-uaf-test.js"], ) +wd_test( + src = "queue-send-redirect-hang-test.wd-test", + args = ["--experimental"], + data = ["queue-send-redirect-hang-test.js"], +) + wd_test( src = "queue-metrics-test.wd-test", args = ["--experimental"], diff --git a/src/workerd/api/tests/queue-send-redirect-hang-test.js b/src/workerd/api/tests/queue-send-redirect-hang-test.js new file mode 100644 index 00000000000..1839d9d2edf --- /dev/null +++ b/src/workerd/api/tests/queue-send-redirect-hang-test.js @@ -0,0 +1,47 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +// Regression test: a fire-and-forget `env.QUEUE.send()` followed by a 3xx +// response used to crash workerd inside `IoContext_IncomingRequest::drain()` +// because the loopback subrequest's `WorkerEntrypoint` was freed while its +// `requestImpl` coroutine was still running. + +import assert from 'node:assert'; + +export default { + async fetch(request, env) { + const { pathname } = new URL(request.url); + + if (pathname === '/message') { + return Response.json({ + metadata: { + metrics: { + backlogCount: 0, + backlogBytes: 0, + oldestMessageTimestamp: 0, + }, + }, + }); + } + + if (pathname === '/queue-then-redirect') { + env.QUEUE.send({ hello: 'world' }); + return new Response(null, { + status: 302, + headers: { location: '/anywhere' }, + }); + } + + return new Response('not found', { status: 404 }); + }, + + async test(_ctrl, env) { + const resp = await env.SERVICE.fetch( + 'http://example.com/queue-then-redirect', + { redirect: 'manual' } + ); + assert.strictEqual(resp.status, 302); + assert.strictEqual(resp.headers.get('location'), '/anywhere'); + }, +}; diff --git a/src/workerd/api/tests/queue-send-redirect-hang-test.wd-test b/src/workerd/api/tests/queue-send-redirect-hang-test.wd-test new file mode 100644 index 00000000000..820c9af0cb4 --- /dev/null +++ b/src/workerd/api/tests/queue-send-redirect-hang-test.wd-test @@ -0,0 +1,18 @@ +using Workerd = import "/workerd/workerd.capnp"; + +const unitTests :Workerd.Config = ( + services = [ + ( name = "queue-send-redirect-hang-test", + worker = ( + modules = [ + ( name = "worker", esModule = embed "queue-send-redirect-hang-test.js" ) + ], + bindings = [ + ( name = "QUEUE", queue = "queue-send-redirect-hang-test" ), + ( name = "SERVICE", service = "queue-send-redirect-hang-test" ), + ], + compatibilityFlags = ["nodejs_compat", "service_binding_extra_handlers", "queues_json_messages"], + ) + ), + ], +); diff --git a/src/workerd/io/worker-entrypoint.c++ b/src/workerd/io/worker-entrypoint.c++ index af3441e1503..be6de163595 100644 --- a/src/workerd/io/worker-entrypoint.c++ +++ b/src/workerd/io/worker-entrypoint.c++ @@ -369,6 +369,16 @@ kj::Promise WorkerEntrypoint::requestImpl(kj::HttpMethod method, TRACE_EVENT("workerd", "WorkerEntrypoint::request()", "url", url.cStr(), PERFETTO_FLOW_FROM_POINTER(this)); + // Bind `waitUntilTasks` to the coroutine frame so the KJ_DEFER below doesn't reach through + // `this`. That DEFER runs from the coroutine's destructor, which on some subrequest paths + // fires after `WorkerEntrypoint` itself has been freed (the promise chain owning the coroutine + // can outlive the `Own` — e.g. a fire-and-forget `env.QUEUE.send()` whose + // caller returned early; see queue-send-redirect-hang-test). Reading `this->waitUntilTasks` + // in that window yields a null reference that faults inside `TaskSet::add`. + // TODO(cleanup): the DEFER still reads other `this->*` members. A proper fix would tie the + // entrypoint's lifetime to the coroutine's rather than defending each field individually. + kj::TaskSet& waitUntilTasks = this->waitUntilTasks; + // ----- Stage 1: Set up per-request state. ----- auto incomingRequest =