Skip to content

Track the ReverseProxy option in the request Scope - #3

Closed
deb-hacktron wants to merge 6 commits into
masterfrom
reverse-proxy-context
Closed

Track the ReverseProxy option in the request Scope#3
deb-hacktron wants to merge 6 commits into
masterfrom
reverse-proxy-context

Conversation

@deb-hacktron

Copy link
Copy Markdown
Owner

Historical PR recreation for internal review-bot demo with dummy-authored commits.

@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.

3 issues found across 2 files

Severity Count
🟡 Medium 3

View full scan results

Comment thread oauthproxy.go
Comment on lines +546 to +563
func (p *OAuthProxy) isTrustedIP(req *http.Request) bool {
if p.trustedIPs == nil {
return false
}

remoteAddr, err := ip.GetClientIP(p.realClientIPParser, req)
if err != nil {
logger.Errorf("Error obtaining real IP for trusted IP list: %v", err)
// Possibly spoofed X-Real-IP header
return false
}

if remoteAddr == nil {
return false
}

return p.trustedIPs.Has(remoteAddr)
}

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: Authentication Bypass via IP Spoofing in Trusted IP Configuration

The isTrustedIP function determines whether a request should bypass authentication based on the client's IP address. It uses ip.GetClientIP(p.realClientIPParser, req) to determine the client's IP. If the proxy is configured with --reverse-proxy=true and --real-client-ip-header=X-Forwarded-For (or similar), it trusts the IP provided in the header. If the proxy is exposed directly to the internet or an untrusted network without a trusted load balancer stripping these headers, an attacker can spoof their IP address by setting the header. This allows them to bypass authentication for any routes protected by IP-based allowlists (--trusted-ip), effectively gaining unauthorized access to internal services. This is a configuration-dependent flaw that requires the proxy to be misconfigured by exposing it directly while trusting proxy headers.

Steps to Reproduce
  1. Configure oauth2-proxy with --reverse-proxy=true, --real-client-ip-header=X-Forwarded-For, and --trusted-ip=192.168.1.100.
  2. Expose oauth2-proxy directly to an untrusted network.
  3. Send a request to a protected route with the header X-Forwarded-For: 192.168.1.100.
  4. Observe that the request bypasses authentication and is proxied to the upstream service.
Trace
graph TD
    subgraph SG0 ["./oauth2-proxy/oauthproxy.go"]
        serveHTTP["serveHTTP"]
        IsAllowedRequest["Checks if the request is allowed without authentication."]
        isTrustedIP{{"Checks if the client IP is in the trusted list."}}
    end
    style SG0 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG1 ["./oauth2-proxy/pkg/logger/logger.go"]
        formatLogMessage["formatLogMessage"]
        Output["Output"]
        GetFileLineString["GetFileLineString"]
        FormatTimestamp["FormatTimestamp"]
        Errorf["Errorf"]
    end
    style SG1 fill:#2a2a2a,stroke:#444,color:#aaa
    isTrustedIP --> Errorf
    Errorf --> Output
    Output --> formatLogMessage
    formatLogMessage --> GetFileLineString
    formatLogMessage --> FormatTimestamp
    FormatTimestamp --> FormatTimestamp
    IsAllowedRequest --> isTrustedIP
    serveHTTP --> IsAllowedRequest
Loading
Fix with AI

Open in Cursor Open in Claude

Fix the following security vulnerability found by Hacktron.

File: oauthproxy.go
Lines: 546-563
Severity: medium

Vulnerability: Authentication Bypass via IP Spoofing in Trusted IP Configuration

Description:
The `isTrustedIP` function determines whether a request should bypass authentication based on the client's IP address. It uses `ip.GetClientIP(p.realClientIPParser, req)` to determine the client's IP. If the proxy is configured with `--reverse-proxy=true` and `--real-client-ip-header=X-Forwarded-For` (or similar), it trusts the IP provided in the header. If the proxy is exposed directly to the internet or an untrusted network without a trusted load balancer stripping these headers, an attacker can spoof their IP address by setting the header. This allows them to bypass authentication for any routes protected by IP-based allowlists (`--trusted-ip`), effectively gaining unauthorized access to internal services. This is a configuration-dependent flaw that requires the proxy to be misconfigured by exposing it directly while trusting proxy headers.

Proof of Concept:
**Steps to Reproduce**

1. Configure oauth2-proxy with `--reverse-proxy=true`, `--real-client-ip-header=X-Forwarded-For`, and `--trusted-ip=192.168.1.100`.
2. Expose oauth2-proxy directly to an untrusted network.
3. Send a request to a protected route with the header `X-Forwarded-For: 192.168.1.100`.
4. Observe that the request bypasses authentication and is proxied to the upstream service.

Affected Code:
func (p *OAuthProxy) isTrustedIP(req *http.Request) bool {
	if p.trustedIPs == nil {
		return false
	}

	remoteAddr, err := ip.GetClientIP(p.realClientIPParser, req)
	if err != nil {
		logger.Errorf("Error obtaining real IP for trusted IP list: %v", err)
		// Possibly spoofed X-Real-IP header
		return false
	}

	if remoteAddr == nil {
		return false
	}

	return p.trustedIPs.Has(remoteAddr)
}

Fix this vulnerability. Only change what's necessary - don't modify unrelated code.

Triage: Reply !fp (false positive), !valid (confirmed), or !accepted_risk. Any other reply is saved as a triage note.

View finding in Hacktron

Comment thread oauthproxy.go
Comment on lines +425 to +466
func (p *OAuthProxy) IsValidRedirect(redirect string) bool {
switch {
case redirect == "":
// The user didn't specify a redirect, should fallback to `/`
return false
case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") && !invalidRedirectRegex.MatchString(redirect):
return true
case strings.HasPrefix(redirect, "http://") || strings.HasPrefix(redirect, "https://"):
redirectURL, err := url.Parse(redirect)
if err != nil {
logger.Printf("Rejecting invalid redirect %q: scheme unsupported or missing", redirect)
return false
}
redirectHostname := redirectURL.Hostname()

for _, domain := range p.whitelistDomains {
domainHostname, domainPort := splitHostPort(strings.TrimLeft(domain, "."))
if domainHostname == "" {
continue
}

if (redirectHostname == domainHostname) || (strings.HasPrefix(domain, ".") && strings.HasSuffix(redirectHostname, domainHostname)) {
// the domain names match, now validate the ports
// if the whitelisted domain's port is '*', allow all ports
// if the whitelisted domain contains a specific port, only allow that port
// if the whitelisted domain doesn't contain a port at all, only allow empty redirect ports ie http and https
redirectPort := redirectURL.Port()
if (domainPort == "*") ||
(domainPort == redirectPort) ||
(domainPort == "" && redirectPort == "") {
return true
}
}
}

logger.Printf("Rejecting invalid redirect %q: domain / port not in whitelist", redirect)
return false
default:
logger.Printf("Rejecting invalid redirect %q: not an absolute or relative URL", redirect)
return false
}
}

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: Open Redirect via Domain Suffix Bypass in IsValidRedirect

The finding correctly identifies that an attacker can bypass the domain check by finding a domain that ends in the whitelisted domain string. When a whitelist domain is configured with a leading dot to allow subdomains (e.g., .example.com), the code strips the leading dot to get domainHostname (e.g., example.com). It then uses strings.HasSuffix(redirectHostname, domainHostname) to validate the redirect domain. If an attacker uses a redirect URL like http://attacker-example.com, strings.HasSuffix("attacker-example.com", "example.com") evaluates to true. This allows an attacker to redirect users to an arbitrary domain they control, leading to a classic Open Redirect vulnerability.

Steps to Reproduce
  1. Configure OAuth2 Proxy with a whitelist domain that includes a leading dot to allow subdomains, e.g., --whitelist-domain=.example.com.
  2. An attacker crafts a malicious link with a redirect parameter pointing to a domain they control that ends with the whitelisted domain string, e.g., https://attacker-example.com.
  3. The victim clicks the link and authenticates.
  4. The IsValidRedirect function strips the leading dot from .example.com to get example.com, and checks if attacker-example.com has the suffix example.com. This evaluates to true.
  5. The victim is successfully redirected to the attacker's domain.
Trace
graph TD
    subgraph SG0 ["./oauth2-proxy/oauthproxy.go"]
        IsValidRedirect{{"IsValidRedirect"}}
        serveHTTP["serveHTTP"]
        SignInPage["Renders the sign-in page."]
        SignIn["Handles the sign-in process."]
        SignOut["Handles user sign-out."]
        OAuthStart["Starts the OAuth2 authentication flow."]
        OAuthCallback["Handles the OAuth2 callback flow."]
        Proxy["Proxies the request to the upstream backend."]
        getAppRedirect["Determines the application redirect URL."]
        validateRedirect["Validates a redirect URL."]
        getRdQuerystringRedirect["Gets redirect from query string."]
        getXAuthRequestRedirect["Gets redirect from header."]
        getXForwardedHeadersRedirect["Gets redirect from forwarded headers."]
        getURIRedirect["Gets redirect from URI."]
        splitHostPort["splitHostPort"]
        validOptionalPort["validOptionalPort"]
    end
    style SG0 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG1 ["./oauth2-proxy/pkg/logger/logger.go"]
        formatLogMessage["formatLogMessage"]
        Output["Output"]
        GetFileLineString["GetFileLineString"]
        FormatTimestamp["FormatTimestamp"]
        Printf["Printf"]
    end
    style SG1 fill:#2a2a2a,stroke:#444,color:#aaa
    IsValidRedirect --> splitHostPort
    IsValidRedirect --> Printf
    splitHostPort --> validOptionalPort
    Printf --> Output
    Output --> formatLogMessage
    formatLogMessage --> GetFileLineString
    formatLogMessage --> FormatTimestamp
    FormatTimestamp --> FormatTimestamp
    OAuthCallback --> IsValidRedirect
    getAppRedirect --> IsValidRedirect
    validateRedirect --> IsValidRedirect
    serveHTTP --> OAuthCallback
    SignInPage --> getAppRedirect
    SignIn --> getAppRedirect
    SignOut --> getAppRedirect
    OAuthStart --> getAppRedirect
    getRdQuerystringRedirect --> validateRedirect
    getXAuthRequestRedirect --> validateRedirect
    getXForwardedHeadersRedirect --> validateRedirect
    getURIRedirect --> validateRedirect
    SignIn --> SignInPage
    Proxy --> SignInPage
    serveHTTP --> SignIn
    serveHTTP --> SignOut
    serveHTTP --> OAuthStart
    SignIn --> OAuthStart
    Proxy --> OAuthStart
    serveHTTP --> Proxy
Loading
Fix with AI

Open in Cursor Open in Claude

Fix the following security vulnerability found by Hacktron.

File: oauthproxy.go
Lines: 425-466
Severity: medium

Vulnerability: Open Redirect via Domain Suffix Bypass in IsValidRedirect

Description:
The finding correctly identifies that an attacker can bypass the domain check by finding a domain that ends in the whitelisted domain string. When a whitelist domain is configured with a leading dot to allow subdomains (e.g., `.example.com`), the code strips the leading dot to get `domainHostname` (e.g., `example.com`). It then uses `strings.HasSuffix(redirectHostname, domainHostname)` to validate the redirect domain. If an attacker uses a redirect URL like `http://attacker-example.com`, `strings.HasSuffix("attacker-example.com", "example.com")` evaluates to `true`. This allows an attacker to redirect users to an arbitrary domain they control, leading to a classic Open Redirect vulnerability.

Proof of Concept:
**Steps to Reproduce**

1. Configure OAuth2 Proxy with a whitelist domain that includes a leading dot to allow subdomains, e.g., `--whitelist-domain=.example.com`.
2. An attacker crafts a malicious link with a redirect parameter pointing to a domain they control that ends with the whitelisted domain string, e.g., `https://attacker-example.com`.
3. The victim clicks the link and authenticates.
4. The `IsValidRedirect` function strips the leading dot from `.example.com` to get `example.com`, and checks if `attacker-example.com` has the suffix `example.com`. This evaluates to true.
5. The victim is successfully redirected to the attacker's domain.

Affected Code:
if (redirectHostname == domainHostname) || (strings.HasPrefix(domain, ".") && strings.HasSuffix(redirectHostname, domainHostname)) {

Fix this vulnerability. Only change what's necessary - don't modify unrelated code.

Triage: Reply !fp (false positive), !valid (confirmed), or !accepted_risk. Any other reply is saved as a triage note.

View finding in Hacktron

Comment on lines +29 to 36
proto := requestutil.GetRequestProto(req)
if strings.EqualFold(proto, httpsScheme) || (req.TLS != nil && proto == req.URL.Scheme) {
// Only care about the connection to us being HTTPS if the proto wasn't
// from a trusted `X-Forwarded-Proto` (proto == req.URL.Scheme).
// Otherwise the proto is source of truth
next.ServeHTTP(rw, req)
return
}

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: Open Redirect via Trusted X-Forwarded-Host in HTTPS Redirect Middleware

The redirectToHTTPS middleware trusts the X-Forwarded-Host header when the --reverse-proxy flag is enabled to construct the target URL for HTTPS redirection. Because it does not validate the host against an allowlist (e.g., using IsValidRedirect), an attacker can supply a malicious X-Forwarded-Host header to redirect users to an arbitrary domain. This allows for Open Redirect attacks if the upstream reverse proxy does not strictly overwrite the X-Forwarded-Host header.

Steps to Reproduce
  1. Run oauth2-proxy with --reverse-proxy enabled and HTTPS redirection active.
  2. Send a request to the proxy over HTTP with a malicious host header:
    curl -i -H "X-Forwarded-Host: evil.com" http://<oauth2-proxy-host>/some-path
  3. Observe the response is a 308 Permanent Redirect to https://evil.com/some-path.
curl -i -H "X-Forwarded-Host: evil.com" http://<oauth2-proxy-host>/some-path
Trace
graph TD
    subgraph SG0 ["./oauth2-proxy/pkg/middleware/redirect_to_https.go"]
        NewRedirectToHTTPS["NewRedirectToHTTPS"]
        redirectToHTTPS{{"Middleware that enforces HTTPS by redirecting non-secure HTTP requests."}}
    end
    style SG0 fill:#2a2a2a,stroke:#444,color:#aaa
    NewRedirectToHTTPS --> redirectToHTTPS
Loading
Fix with AI

Open in Cursor Open in Claude

Fix the following security vulnerability found by Hacktron.

File: pkg/middleware/redirect_to_https.go
Lines: 29-45
Severity: medium

Vulnerability: Open Redirect via Trusted X-Forwarded-Host in HTTPS Redirect Middleware

Description:
The `redirectToHTTPS` middleware trusts the `X-Forwarded-Host` header when the `--reverse-proxy` flag is enabled to construct the target URL for HTTPS redirection. Because it does not validate the host against an allowlist (e.g., using `IsValidRedirect`), an attacker can supply a malicious `X-Forwarded-Host` header to redirect users to an arbitrary domain. This allows for Open Redirect attacks if the upstream reverse proxy does not strictly overwrite the `X-Forwarded-Host` header.

Proof of Concept:
**Steps to Reproduce**

1. Run oauth2-proxy with `--reverse-proxy` enabled and HTTPS redirection active.
2. Send a request to the proxy over HTTP with a malicious host header:
   `curl -i -H "X-Forwarded-Host: evil.com" http://<oauth2-proxy-host>/some-path`
3. Observe the response is a 308 Permanent Redirect to `https://evil.com/some-path`.

```bash
curl -i -H "X-Forwarded-Host: evil.com" http://<oauth2-proxy-host>/some-path
```

Affected Code:
./oauth2-proxy/pkg/middleware/redirect_to_https.go:27-56

Fix this vulnerability. Only change what's necessary - don't modify unrelated code.

Triage: Reply !fp (false positive), !valid (confirmed), or !accepted_risk. Any other reply is saved as a triage note.

View finding in Hacktron

@deb-hacktron

Copy link
Copy Markdown
Owner Author

Refreshing scan with a new PR.

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