Description
Filter::addDownstreamSetCookie() in source/common/router/router.h (line 500-501 at current main) dereferences downstreamConnection() unconditionally to compose the per-connection seed used in the generated cookie value:
const Network::Connection* conn = downstreamConnection();
// Need to check for null conn if this is ever used by Http::AsyncClient in the future.
value = conn->connectionInfoProvider().remoteAddress()->asString() +
conn->connectionInfoProvider().localAddress()->asString();
When the router is invoked without a downstream connection — Http::AsyncClient-driven paths such as ext_authz or ratelimit side calls, mirror/shadow, and health checks — conn is null and the worker process crashes (SIGSEGV) whenever the route is configured with a cookie hash policy.
The TODO comment on line 500 explicitly flagged the risk; the null deref is reachable today from any deployment that combines a cookie hash policy with an async-router-driven side call.
Reproduction
- Configure a route with a cookie hash policy on the cluster (or a typed hash policy that derives a cookie value).
- Route requests through that cluster via an async-router path that doesn't carry a downstream connection (e.g.
ext_authz HTTP/gRPC side call, ratelimit side call, mirror/shadow).
- The first request that traverses
addDownstreamSetCookie() crashes the worker.
Severity / scope
CWE-476 null-pointer dereference; worker-process crash; denial of service. Not a regression of GHSA-fp35-g349-h66f (CVE-2024-39305) — that was a UAF on cookie attribute lifetime at config-parse time; this is a separate request-time null dereference in a different file and a different code path.
Proposed fix
Add a null check and return an empty cookie value when no downstream connection is available, which causes the cookie hash policy to skip the connection-derived seed for those callers. The resulting cookie hashes identically across all async invocations, preserving hash-stability for the policy. Pull request follows.
Background
This was first reported privately as GHSA-47qr-fw7j-6m57. The envoy security team (@ggreenway, @paul-r-gall) asked for this to be fixed in the open as a non-security bug because the misconfiguration would surface in basic functional testing, so I'm filing here per their redirect.
Description
Filter::addDownstreamSetCookie()insource/common/router/router.h(line 500-501 at current main) dereferencesdownstreamConnection()unconditionally to compose the per-connection seed used in the generated cookie value:When the router is invoked without a downstream connection —
Http::AsyncClient-driven paths such asext_authzor ratelimit side calls, mirror/shadow, and health checks —connis null and the worker process crashes (SIGSEGV) whenever the route is configured with a cookie hash policy.The TODO comment on line 500 explicitly flagged the risk; the null deref is reachable today from any deployment that combines a cookie hash policy with an async-router-driven side call.
Reproduction
ext_authzHTTP/gRPC side call, ratelimit side call, mirror/shadow).addDownstreamSetCookie()crashes the worker.Severity / scope
CWE-476 null-pointer dereference; worker-process crash; denial of service. Not a regression of GHSA-fp35-g349-h66f (CVE-2024-39305) — that was a UAF on cookie attribute lifetime at config-parse time; this is a separate request-time null dereference in a different file and a different code path.
Proposed fix
Add a null check and return an empty cookie value when no downstream connection is available, which causes the cookie hash policy to skip the connection-derived seed for those callers. The resulting cookie hashes identically across all async invocations, preserving hash-stability for the policy. Pull request follows.
Background
This was first reported privately as
GHSA-47qr-fw7j-6m57. The envoy security team (@ggreenway, @paul-r-gall) asked for this to be fixed in the open as a non-security bug because the misconfiguration would surface in basic functional testing, so I'm filing here per their redirect.