ReviewOS

stacks/rpx

HTTP proxy path sets x-real-ip but not x-forwarded-host / -for / -proto, while rewriting Host

#2265
Closed glennmichael123 opened this 10 days ago ·
10 days ago

Summary

When rpx proxies an ordinary HTTP request it rewrites Host to the upstream target but forwards only x-real-ip. x-forwarded-host, x-forwarded-for and x-forwarded-proto never reach the app, so an application behind rpx has no way to learn the hostname or scheme the client actually used.

The WebSocket upgrade path in the same file sets all four.

Observed on @stacksjs/rpx@0.11.46.

What the upstream actually receives

Captured on the loopback interface between rpx and the app (tcpdump -i lo -A 'tcp port 3022') while making one HTTPS request to a proxied host:

host:        localhost:3022
origin:      https://bughq.org
x-real-ip:   ::ffff:127.0.0.1

No x-forwarded-host, no x-forwarded-for, no x-forwarded-proto.

The site fragment is the ordinary shape:

{ "to": "bughq.org", "from": "localhost:3022", "id": "bughq.org" }

Why it matters

Any check that needs the original host cannot work. The concrete case: a CSRF origin guard comparing the browser's Origin against the request host.

const host = req.headers.get('x-forwarded-host') || req.headers.get('host')
return new URL(origin).host === host        // 'bughq.org' vs 'localhost:3022'

That returns false for every legitimate browser request, so the endpoint answered 403 for every user in production. Reproducible from outside:

curl -X POST -H 'Origin: https://bughq.org' https://bughq.org/api/...   -> 403
curl -X POST                                https://bughq.org/api/...   -> 401

The 403/401 split is the tell: with no Origin the guard is skipped and the request reaches auth; with one, it cannot pass. Requests sent straight to the app with Host: bughq.org return 401, so the application logic is fine and only the proxied path fails.

x-forwarded-proto has the same problem for anything generating absolute URLs or deciding whether to force HTTPS: the app cannot tell that the client arrived over TLS, because rpx terminates it.

The inconsistency

Both header sets exist in dist. The WebSocket upgrade path sets all four:

T.host = v
T['x-forwarded-for'] = l
T['x-real-ip'] = l
T['x-forwarded-proto'] = 'https'
T['x-forwarded-host'] = c

and there is an HTTP-side branch that does the equivalent with .set(), guarded by a condition that the plain proxy path does not appear to reach. From the outside it looks like the headers were added with WebSocket support (#26) and the ordinary path was left behind.

Incoming x-forwarded-* are already stripped as hop-by-hop:

new Set(['host','connection','keep-alive','proxy-connection','transfer-encoding',
         'x-forwarded-for','x-forwarded-proto','x-forwarded-host','x-real-ip'])

which is the right thing to do and means setting them unconditionally afterwards is safe: a client cannot spoof what the proxy overwrites.

Suggested fix

On the HTTP proxy path, after the hop-by-hop strip, set the same four the WebSocket path already does: host (upstream), x-forwarded-host (the original Host), x-forwarded-for / x-real-ip (client address), and x-forwarded-proto. That matches nginx/caddy/traefik defaults and is what applications behind a reverse proxy expect.

Reproduction

  1. Front any HTTP server with rpx using a normal { to, from } fragment.
  2. Have the upstream echo its request headers.
  3. Request the proxied hostname over HTTPS.
  4. Only x-real-ip is present; Host is the upstream target.
10 days ago

Not reproducible on current main — and covered now so it can't regress silently.

Ran a live echo upstream through createProxyFetchHandler, which every entry point (startProxy, the daemon, the gateway) funnels into. All four headers arrive on all three transports:

transporthostx-real-ipx-forwarded-forx-forwarded-hostx-forwarded-proto
pooled (ordinary GET / POST)upstreamclientclientoriginalhttps
fetch fallback (SSE etc.)upstreamclientclientoriginalhttps
websocket upgradeupstreamclientclientoriginalhttps

Client-supplied copies of all four are stripped first (STRIP_REQUEST in proxy-pool.ts), then rpx's own values are written unconditionally — so a client can't spoof them, exactly as the issue suggests.

The coverage gap was real though: x-forwarded-proto was asserted nowhere, and nothing covered the handler layer end to end — only the pool serializer beneath it. #2268 pins both transports at the handler boundary and adds the missing proto assertion. I confirmed they're genuine guards by reintroducing the described bug in serializeRequest: 8 cases fail, including the new one.

One adjacent finding while in here, filed separately: x-forwarded-proto is the literal https on all three paths, so in --no-https / plain-HTTP single-port mode the upstream is told the client used TLS when it didn't. Bun's req.url carries the real scheme, so that's a straightforward fix — but it's a different bug from this one.

Closing; the tests land with #2268.

Sign in to comment on this issue.