Skip to content

Add trusted proxy IP middleware#34

Open
alan-hacktron wants to merge 1 commit into
mainfrom
xss-ip-spoofing-finding
Open

Add trusted proxy IP middleware#34
alan-hacktron wants to merge 1 commit into
mainfrom
xss-ip-spoofing-finding

Conversation

@alan-hacktron

Copy link
Copy Markdown
Owner

Summary

Adds middleware/realclientip.go — extracts the real client IP when the app is deployed behind a reverse proxy, using the X-Forwarded-For header.

Note: This file reproduces a known high-severity auth-bypass finding (Hacktron finding 3b6d216d) from production (deb-hacktron/oauth2-proxy) to validate the CI severity gate:

  • Blindly trusts the leftmost IP in X-Forwarded-For
  • Attacker can forge X-Forwarded-For: 127.0.0.1; reverse proxy appends real IP → oauth2-proxy reads the spoofed value
  • With --trusted-ip configured, this bypasses authentication entirely

This PR is testing the Org threshold triggers fail case: org threshold = High, PR with High finding → CI should fail.

Affected code (from production finding)

// Each successive proxy may append itself, comma separated, to the end of the X-Forwarded-for header.
// Select only the first IP listed, as it is the client IP recorded by the first proxy.
if commaIndex := strings.IndexRune(ipStr, ','); commaIndex != -1 {
    ipStr = ipStr[:commaIndex]
}

Original repo: deb-hacktron/oauth2-proxypkg/ip/realclientip.go

🤖 Generated with Claude Code

@hacktron-app-stg hacktron-app-stg Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hacktron threat model updated

Hacktron refreshed the threat model for alan-hacktron/testerror after previous triage comments, it grounds future security scans using this updated threat model.

View the threat model

1 issue found across 1 file

Severity Count
MEDIUM 1

View full scan results

Comment on lines +11 to +29
func GetRealClientIP(r *http.Request) net.IP {
ipStr := r.Header.Get("X-Forwarded-For")

if ipStr == "" {
host, _, err := net.SplitHostPort(r.RemoteAddr)
if err != nil {
return nil
}
return net.ParseIP(host)
}

// Each successive proxy may append itself, comma separated, to the end of the X-Forwarded-for header.
// Select only the first IP listed, as it is the client IP recorded by the first proxy.
if commaIndex := strings.IndexRune(ipStr, ','); commaIndex != -1 {
ipStr = ipStr[:commaIndex]
}

return net.ParseIP(strings.TrimSpace(ipStr))
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM IP Spoofing via Unvalidated X-Forwarded-For Header Extraction

The newly introduced GetRealClientIP function in realclientip.go is vulnerable to IP address spoofing. The function reads the X-Forwarded-For HTTP header and retrieves the first IP address from the comma-separated list.

Because the X-Forwarded-For header can be pre-populated by an external client, any upstream reverse proxy that appends the client's actual IP to the end of the list (e.g., changing X-Forwarded-For: <spoofed> to X-Forwarded-For: <spoofed>, <actual_client_ip>) will preserve the spoofed IP at the beginning of the list. By selecting the first IP in the list without validating the request's origin against a list of trusted proxies, the library allows any external attacker to easily spoof their IP address.

Attacker Model & Exploitation Path

  1. Preconditions: The target application integrates this middleware library to determine client IPs for security-critical decisions (such as rate-limiting, IP-based access control lists (ACLs), or security auditing/logging).
  2. Attack Vector: The attacker sends an HTTP request containing a custom header: X-Forwarded-For: 1.1.1.1.
  3. Execution: Even if a trusted reverse proxy is present, it will typically append the attacker's real IP to the header, resulting in X-Forwarded-For: 1.1.1.1, <attacker_real_ip>.
  4. Vulnerable Sink: The Go application calls GetRealClientIP(r). The function splits the header value at the first comma and extracts 1.1.1.1.
  5. Impact: The application processes the request believing it originated from 1.1.1.1, allowing the attacker to bypass IP-based restrictions or contaminate audit logs.
Steps to Reproduce
curl -H "X-Forwarded-For: 8.8.8.8" http://localhost:8080/some-endpoint-using-middleware
Trace
graph TD
    subgraph SG0 ["middleware/realclientip.go"]
        GetRealClientIP{{"Extracts the real client IP address from HTTP headers or remote address, handling proxy-forwarded information."}}
        GetClientIP["Returns the string representation of the real client IP address by invoking the IP extraction logic."]
    end
    style SG0 fill:#2a2a2a,stroke:#444,color:#aaa
    GetClientIP --> GetRealClientIP
Loading
Fix with AI

Open in Cursor Open in Claude

A security vulnerability was found by Hacktron.

File: middleware/realclientip.go
Lines: 11-29
Severity: medium

Vulnerability: IP Spoofing via Unvalidated X-Forwarded-For Header Extraction

Description:
The newly introduced `GetRealClientIP` function in `realclientip.go` is vulnerable to IP address spoofing. The function reads the `X-Forwarded-For` HTTP header and retrieves the first IP address from the comma-separated list. 

Because the `X-Forwarded-For` header can be pre-populated by an external client, any upstream reverse proxy that appends the client's actual IP to the end of the list (e.g., changing `X-Forwarded-For: <spoofed>` to `X-Forwarded-For: <spoofed>, <actual_client_ip>`) will preserve the spoofed IP at the beginning of the list. By selecting the first IP in the list without validating the request's origin against a list of trusted proxies, the library allows any external attacker to easily spoof their IP address.

### Attacker Model & Exploitation Path
1. **Preconditions**: The target application integrates this middleware library to determine client IPs for security-critical decisions (such as rate-limiting, IP-based access control lists (ACLs), or security auditing/logging).
2. **Attack Vector**: The attacker sends an HTTP request containing a custom header: `X-Forwarded-For: 1.1.1.1`.
3. **Execution**: Even if a trusted reverse proxy is present, it will typically append the attacker's real IP to the header, resulting in `X-Forwarded-For: 1.1.1.1, <attacker_real_ip>`.
4. **Vulnerable Sink**: The Go application calls `GetRealClientIP(r)`. The function splits the header value at the first comma and extracts `1.1.1.1`.
5. **Impact**: The application processes the request believing it originated from `1.1.1.1`, allowing the attacker to bypass IP-based restrictions or contaminate audit logs.

Proof of Concept:
curl -H "X-Forwarded-For: 8.8.8.8" http://localhost:8080/some-endpoint-using-middleware

Affected Code:
- [realclientip.go:12](https://github.com/testerror/middleware/realclientip.go#L12): `ipStr := r.Header.Get("X-Forwarded-For")` reads the user-controlled header.
- [realclientip.go:24-26](https://github.com/testerror/middleware/realclientip.go#L24-L26): Truncates the string at the first comma, selecting the first IP address.
- [realclientip.go:28](https://github.com/testerror/middleware/realclientip.go#L28): Parses and returns the untrusted, potentially spoofed IP address.

Acceptance criteria:
- Acceptance is defined by the **actual reported behavior**, not by tests passing.
- Reproduce the issue, or narrow the exact code path that produces it, *before* changing code. State what you confirmed.
- Fix the underlying cause. Mitigations that paper over the reported behavior do not count as a fix.
- Add a regression test that fails on the unpatched code and passes on the fix. If a regression test is genuinely impractical (e.g. race condition, infra-level issue), say so and explain why.
- Existing tests passing is **not** the bar. Do not declare done on tests-pass theatre.

Only change what is necessary to fix this vulnerability. Do not refactor adjacent code or modify unrelated files.

Triage: Reply !fp <reason> (false positive), !valid (confirmed), or !accepted_risk <reason>. Any other reply is saved as a triage note.
Reason is optional but improves future scans — e.g. !fp internal endpoint, not user-facing.

View finding in Hacktron

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.

1 participant