Skip to content

Track the ReverseProxy option in the request Scope - #10

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

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

Conversation

@deb-hacktron

Copy link
Copy Markdown
Owner

Fresh PR to rerun Hacktron after pipeline changes.

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

1 issue found across 1 file

Severity Count
🟡 Medium 1

View full scan results

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 is designed to redirect HTTP requests to HTTPS. It constructs the target URL by copying the original request URL, changing the scheme to https, and setting the host using requestutil.GetRequestHost(req).

When the proxy is configured with --reverse-proxy (which sets the ReverseProxy scope to true), GetRequestHost prioritizes the X-Forwarded-Host header over the standard Host header. Because redirectToHTTPS does not validate the resulting host against any whitelist (unlike other redirect logic in the application, such as IsValidRedirect), an attacker can supply an arbitrary domain in the X-Forwarded-Host header.

While an attacker cannot directly force a victim's browser to send a custom X-Forwarded-Host header, the redirect uses a 308 Permanent Redirect (http.StatusPermanentRedirect), which is cacheable by default. If the proxy is deployed behind a CDN or a caching reverse proxy, an attacker can send a crafted request with a malicious X-Forwarded-Host header. The caching layer may cache the resulting 308 redirect and serve it to subsequent legitimate users, leading to a Web Cache Poisoning attack that results in a widespread Open Redirect.

Steps to Reproduce
  1. Start oauth2-proxy with --force-https and --reverse-proxy enabled.
  2. Send an HTTP request with a malicious X-Forwarded-Host header:
    curl -i -H "Host: legitimate.com" -H "X-Forwarded-Host: evil.com" http://<proxy-address>/
  3. Observe the response, which redirects to the attacker-controlled domain:
    HTTP/1.1 308 Permanent Redirect
    Location: https://evil.com/
  4. If a caching layer (CDN/Proxy) sits in front of the application, this response may be cached for the cache key http://legitimate.com/, causing all subsequent visitors to be redirected to https://evil.com/.
curl -i -H "Host: legitimate.com" -H "X-Forwarded-Host: evil.com" http://<proxy-address>/
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 is designed to redirect HTTP requests to HTTPS. It constructs the target URL by copying the original request URL, changing the scheme to `https`, and setting the host using `requestutil.GetRequestHost(req)`. 

When the proxy is configured with `--reverse-proxy` (which sets the `ReverseProxy` scope to true), `GetRequestHost` prioritizes the `X-Forwarded-Host` header over the standard `Host` header. Because `redirectToHTTPS` does not validate the resulting host against any whitelist (unlike other redirect logic in the application, such as `IsValidRedirect`), an attacker can supply an arbitrary domain in the `X-Forwarded-Host` header. 

While an attacker cannot directly force a victim's browser to send a custom `X-Forwarded-Host` header, the redirect uses a `308 Permanent Redirect` (`http.StatusPermanentRedirect`), which is cacheable by default. If the proxy is deployed behind a CDN or a caching reverse proxy, an attacker can send a crafted request with a malicious `X-Forwarded-Host` header. The caching layer may cache the resulting 308 redirect and serve it to subsequent legitimate users, leading to a Web Cache Poisoning attack that results in a widespread Open Redirect.

Proof of Concept:
**Steps to Reproduce**

1. Start `oauth2-proxy` with `--force-https` and `--reverse-proxy` enabled.
2. Send an HTTP request with a malicious `X-Forwarded-Host` header:
   ```bash
   curl -i -H "Host: legitimate.com" -H "X-Forwarded-Host: evil.com" http://<proxy-address>/
   ```
3. Observe the response, which redirects to the attacker-controlled domain:
   ```http
   HTTP/1.1 308 Permanent Redirect
   Location: https://evil.com/
   ```
4. If a caching layer (CDN/Proxy) sits in front of the application, this response may be cached for the cache key `http://legitimate.com/`, causing all subsequent visitors to be redirected to `https://evil.com/`.

```bash
curl -i -H "Host: legitimate.com" -H "X-Forwarded-Host: evil.com" http://<proxy-address>/
```

Affected Code:
func redirectToHTTPS(httpsPort string, next http.Handler) http.Handler {
	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
		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 the request URL
		targetURL, _ := url.Parse(req.URL.String())
		// Set the scheme to HTTPS
		targetURL.Scheme = httpsScheme

		// Set the Host in case the targetURL still does not have one
		// or it isn't X-Forwarded-Host aware
		targetURL.Host = requestutil.GetRequestHost(req)

		// Overwrite the port if the original request was to a non-standard port
		if targetURL.Port() != "" {
			// If Port was not empty, this should be fine to ignore the error
			host, _, _ := net.SplitHostPort(targetURL.Host)
			targetURL.Host = net.JoinHostPort(host, httpsPort)
		}

		http.Redirect(rw, req, targetURL.String(), http.StatusPermanentRedirect)
	})
}

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