Skip to content

fix: guard reads in the dev server against DNS rebinding - #744

Open
prasanna8585 wants to merge 2 commits into
google:mainfrom
prasanna8585:fix/dns-rebinding-guard-dev-server
Open

fix: guard reads in the dev server against DNS rebinding#744
prasanna8585 wants to merge 2 commits into
google:mainfrom
prasanna8585:fix/dns-rebinding-guard-dev-server

Conversation

@prasanna8585

Copy link
Copy Markdown
Contributor

The dev server (adk web / api_server) applied CORS only when an operator explicitly configured --allow_origins, and had no Host-header validation anywhere. A page reached by rebinding an attacker-controlled hostname to 127.0.0.1 is same-origin as far as the browser is concerned, so it omits Origin -- meaning every read endpoint (GET /list-apps, /version, and others) was reachable from any external page a developer running the dev server happened to visit, with no explicit configuration required to be vulnerable.

This matches the fix adk-python shipped the same day (2cf4fd1) for the identical bug in the identical framework.

Adds isDnsRebindingRequest, checking the Host header against the server's actual bind host (not a client-suppliable header) and any operator-configured --allow_origins host. Applied as the very first middleware, before any route including /health and /version, and on every method (not just state-changing ones) since Origin cannot be relied on for a same-origin-looking rebound request.

Dynamically confirmed with node:http's raw client -- fetch() silently rewrites Host to match the real connection target for any request it dispatches, so it can't be used to reproduce what a rebound page's browser actually sends on the wire. Added three regression tests: accepts a request whose Host names the loopback bind, rejects a GET whose Host does not, and rejects a read endpoint with no Origin header at all (the exact shape of a DNS-rebound request). Full adk_api_server_test.ts suite (70 tests) passes.

The dev server (adk web / api_server) applied CORS only when an
operator explicitly configured --allow_origins, and had no
Host-header validation anywhere. A page reached by rebinding an
attacker-controlled hostname to 127.0.0.1 is same-origin as far as
the browser is concerned, so it omits Origin -- meaning every read
endpoint (GET /list-apps, /version, and others) was reachable from
any external page a developer running the dev server happened to
visit, with no explicit configuration required to be vulnerable.

Adds isDnsRebindingRequest, checking the Host header against the
server's actual bind host (not a client-suppliable header) and any
operator-configured --allow_origins host. Applied as the very first
middleware, before any route including /health and /version, and on
every method (not just state-changing ones) since Origin cannot be
relied on for a same-origin-looking rebound request.

Matches the fix adk-python shipped the same day for the identical
bug in the identical framework.

Dynamically confirmed with node:http's raw client (fetch() silently
rewrites Host to match the real connection target for any request it
dispatches, so it cannot be used to reproduce what a rebound page's
browser actually sends on the wire). Adds three regression tests:
accepts a request whose Host names the loopback bind, rejects a GET
whose Host does not, and rejects a read endpoint with no Origin
header at all -- the exact shape of a DNS-rebound request. Full
adk_api_server_test.ts suite (70 tests) passes.
Comment thread dev/src/server/adk_api_server.ts Outdated
Per review: isLoopbackAddress, getAllowedRequestHosts, and
isDnsRebindingRequest now live in dns_rebinding_guard.ts instead of
adk_api_server.ts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants