extract_client_ip trusts X-Forwarded-For from any peer (spoofing risk with IP auto-login) #32

New issue

Open

opened 2026-04-26 15:53:32 +00:00 by sameh-farouk · 0 comments

sameh-farouk commented

2026-04-26 15:53:32 +00:00

Member

Problem

extract_client_ip (in crates/hero_proxy_server/src/proxy.rs) honors X-Real-Ip and X-Forwarded-For from any source, with no validation that the immediate peer is a trusted upstream proxy. With IP auto-login (user_networks) active, this means anyone who can reach a hero_proxy listener directly can spoof their source IP via X-Forwarded-For and impersonate whatever user that IP is mapped to.

Severity

High once IP auto-login is wired up (currently broken for direct connections — see #31 for the fix that makes it work, which is what surfaces this gap operationally). Concrete attack:

# Operator has user_networks row: 127.0.0.1/32 → user_id=1 (admin)
# Attacker reaches the listener from any IP:
curl -H "X-Forwarded-For: 127.0.0.1" \
     http://<proxy-host>:9997/hero_collab/rpc/rpc \
     -d '{"method":"workspace.delete","params":{"id":1}}'
# Proxy: extract_client_ip reads X-Forwarded-For first → "127.0.0.1"
#   → find_user_for_ip(127.0.0.1) → user 1 (admin)
#   → injects X-Hero-User: <admin>
#   → collab in proxy mode treats request as admin

This pre-dates the peer-addr fallback fix (the trust gap was always there) but was operationally invisible because IP-auto-login itself was broken from browser → proxy.

Fix shape

Add a trusted_proxies setting (DB column on settings table or env var):

trusted_proxies = [] (default) → edge mode. Ignore X-Real-Ip and X-Forwarded-For entirely. Only use TCP peer addr.
trusted_proxies = [10.0.0.0/8, ...] → behind-CDN mode. Honor X-Real-Ip / X-Forwarded-For ONLY when the immediate peer (TCP source) is in that list.

Standard pattern — matches nginx's set_real_ip_from, Cloudflare's trusted IP ranges, tower-http's RealIp middleware design.

Implementation:

fn extract_client_ip(
    req: &Request<Body>,
    peer_addr: Option<&SocketAddr>,
    trusted_proxies: &[IpNetwork],  // new
) -> Option<String> {
    let peer_trusted = peer_addr
        .map(|p| trusted_proxies.iter().any(|net| net.contains(p.ip())))
        .unwrap_or(false);

    if peer_trusted {
        // honor X-Real-Ip / X-Forwarded-For
        ...
    }
    // fall back to peer_addr
    peer_addr.map(|a| a.ip().to_string())
}

Plus admin UI for managing the trusted list.

Backward compat

Existing deployments behind nginx/caddy: operator adds the upstream proxy's IP to trusted_proxies. One-time setup per deployment.
Edge deployments (default): no change to behavior — X-Forwarded-For is silently ignored, peer addr always wins. Most secure default.

## Problem `extract_client_ip` (in `crates/hero_proxy_server/src/proxy.rs`) honors `X-Real-Ip` and `X-Forwarded-For` from any source, with no validation that the immediate peer is a trusted upstream proxy. With IP auto-login (`user_networks`) active, this means anyone who can reach a hero_proxy listener directly can spoof their source IP via `X-Forwarded-For` and impersonate whatever user that IP is mapped to. ## Severity High once IP auto-login is wired up (currently broken for direct connections — see #31 for the fix that makes it work, which is what surfaces this gap operationally). Concrete attack: ```bash # Operator has user_networks row: 127.0.0.1/32 → user_id=1 (admin) # Attacker reaches the listener from any IP: curl -H "X-Forwarded-For: 127.0.0.1" \ http://<proxy-host>:9997/hero_collab/rpc/rpc \ -d '{"method":"workspace.delete","params":{"id":1}}' # Proxy: extract_client_ip reads X-Forwarded-For first → "127.0.0.1" # → find_user_for_ip(127.0.0.1) → user 1 (admin) # → injects X-Hero-User: <admin> # → collab in proxy mode treats request as admin ``` This pre-dates the peer-addr fallback fix (the trust gap was always there) but was operationally invisible because IP-auto-login itself was broken from browser → proxy. ## Fix shape Add a `trusted_proxies` setting (DB column on `settings` table or env var): - `trusted_proxies = []` (default) → edge mode. Ignore `X-Real-Ip` and `X-Forwarded-For` entirely. Only use TCP peer addr. - `trusted_proxies = [10.0.0.0/8, ...]` → behind-CDN mode. Honor `X-Real-Ip` / `X-Forwarded-For` ONLY when the immediate peer (TCP source) is in that list. Standard pattern — matches nginx's `set_real_ip_from`, Cloudflare's [trusted IP ranges](https://www.cloudflare.com/ips/), tower-http's `RealIp` middleware design. Implementation: ```rust fn extract_client_ip( req: &Request<Body>, peer_addr: Option<&SocketAddr>, trusted_proxies: &[IpNetwork], // new ) -> Option<String> { let peer_trusted = peer_addr .map(|p| trusted_proxies.iter().any(|net| net.contains(p.ip()))) .unwrap_or(false); if peer_trusted { // honor X-Real-Ip / X-Forwarded-For ... } // fall back to peer_addr peer_addr.map(|a| a.ip().to_string()) } ``` Plus admin UI for managing the trusted list. ## Backward compat - Existing deployments behind nginx/caddy: operator adds the upstream proxy's IP to `trusted_proxies`. One-time setup per deployment. - Edge deployments (default): no change to behavior — `X-Forwarded-For` is silently ignored, peer addr always wins. Most secure default.