Skip to content

Add HealthCheck middleware - #18

Closed
deb-hacktron wants to merge 9 commits into
base/pr-620from
healthcheck-middleware
Closed

Add HealthCheck middleware#18
deb-hacktron wants to merge 9 commits into
base/pr-620from
healthcheck-middleware

Conversation

@deb-hacktron

Copy link
Copy Markdown
Owner

Fresh PR to rerun Hacktron for the HealthCheck case only.

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

5 issues found across 3 files

Severity Count
🚨 Critical 1
🔴 High 1
🟡 Medium 3

View full scan results

Comment on lines +40 to +48
func isHealthCheckRequest(paths, userAgents map[string]struct{}, req *http.Request) bool {
if _, ok := paths[req.URL.EscapedPath()]; ok {
return true
}
if _, ok := userAgents[req.Header.Get("User-Agent")]; ok {
return true
}
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.

🚨 Critical: Authentication Bypass in Forward Auth Mode via Health Check User-Agent

The HealthCheck middleware in pkg/middleware/healthcheck.go intercepts requests and returns a 200 OK response if the request's User-Agent matches the configured PingUserAgent. By default, PingUserAgent is an empty string, meaning any request without a User-Agent header will trigger this health check response. Because the HealthCheck middleware is placed at the very beginning of the HTTP handler chain (before authentication in oauthproxy), it bypasses all authentication checks. In an Nginx auth_request (Forward Auth) deployment, Nginx relies on the proxy returning a 2xx status code to authorize access to the upstream application. An attacker can simply omit the User-Agent header (or spoof the configured PingUserAgent, such as GoogleHC/1.0 if GCP health checks are enabled) to force the proxy to return 200 OK. This tricks Nginx into believing the request is authenticated, granting the attacker unauthorized access to the protected upstream application.

Steps to Reproduce
  1. Configure oauth2-proxy in Forward Auth mode (e.g., with Nginx using auth_request).
  2. Send a request to the protected Nginx endpoint without a User-Agent header: curl -H "User-Agent:" http://protected-app.example.com/
  3. Nginx forwards the auth request to oauth2-proxy.
  4. oauth2-proxy's HealthCheck middleware matches the empty User-Agent and returns 200 OK.
  5. Nginx interprets the 200 OK as a successful authentication and forwards the original request to the upstream application, bypassing all authentication.
Trace
graph TD
    subgraph SG0 ["./oauth2-proxy/main.go"]
        main["Main entry point for the application, handles configuration and server initialization."]
    end
    style SG0 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG1 ["./oauth2-proxy/pkg/middleware/healthcheck.go"]
        NewHealthCheck["Constructs a health check middleware constructor."]
        healthCheck["Middleware handler that intercepts requests to check if they are health check requests."]
        isHealthCheckRequest{{"Determines if a request matches the configured health check paths or user agents."}}
    end
    style SG1 fill:#2a2a2a,stroke:#444,color:#aaa
    healthCheck --> isHealthCheckRequest
    NewHealthCheck --> healthCheck
    main --> NewHealthCheck
Loading
Fix with AI

Open in Cursor Open in Claude

Fix the following security vulnerability found by Hacktron.

File: pkg/middleware/healthcheck.go
Lines: 40-48
Severity: critical

Vulnerability: Authentication Bypass in Forward Auth Mode via Health Check User-Agent

Description:
The `HealthCheck` middleware in `pkg/middleware/healthcheck.go` intercepts requests and returns a `200 OK` response if the request's `User-Agent` matches the configured `PingUserAgent`. By default, `PingUserAgent` is an empty string, meaning any request without a `User-Agent` header will trigger this health check response. Because the `HealthCheck` middleware is placed at the very beginning of the HTTP handler chain (before authentication in `oauthproxy`), it bypasses all authentication checks. In an Nginx `auth_request` (Forward Auth) deployment, Nginx relies on the proxy returning a 2xx status code to authorize access to the upstream application. An attacker can simply omit the `User-Agent` header (or spoof the configured `PingUserAgent`, such as `GoogleHC/1.0` if GCP health checks are enabled) to force the proxy to return `200 OK`. This tricks Nginx into believing the request is authenticated, granting the attacker unauthorized access to the protected upstream application.

Proof of Concept:
**Steps to Reproduce**

1. Configure oauth2-proxy in Forward Auth mode (e.g., with Nginx using `auth_request`).
2. Send a request to the protected Nginx endpoint without a `User-Agent` header: `curl -H "User-Agent:" http://protected-app.example.com/`
3. Nginx forwards the auth request to oauth2-proxy.
4. oauth2-proxy's `HealthCheck` middleware matches the empty `User-Agent` and returns `200 OK`.
5. Nginx interprets the `200 OK` as a successful authentication and forwards the original request to the upstream application, bypassing all authentication.

Affected Code:
func isHealthCheckRequest(paths, userAgents map[string]struct{}, req *http.Request) bool {
	if _, ok := paths[req.URL.EscapedPath()]; ok {
		return true
	}
	if _, ok := userAgents[req.Header.Get("User-Agent")]; ok {
		return true
	}
	return false
}

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 pkg/validation/options.go
msgs = parseSignatureKey(o, msgs)
msgs = validateCookieName(o, msgs)
msgs = configureLogger(o.Logging, o.PingPath, msgs)
msgs = configureLogger(o.Logging, msgs)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 High: Global Insecure TLS Configuration via http.DefaultClient Modification

The application modifies the global http.DefaultClient when the --ssl-insecure-skip-verify flag is enabled. In Go, http.DefaultClient is a shared global variable used by many functions (e.g., http.Get, http.Post). Modifying this variable affects all outgoing HTTPS requests made by the entire application process, not just those intended for the OAuth2 provider. This creates a global risk of Man-in-the-Middle (MitM) attacks for any HTTPS communication performed by the proxy, including potentially sensitive traffic to other services or dependencies.

Steps to Reproduce
  1. Configure the application to run with the --ssl-insecure-skip-verify flag enabled.
  2. The application's Validate function executes, which overwrites the global http.DefaultClient with a new http.Client configured with InsecureSkipVerify: true.
  3. Any subsequent outgoing HTTPS request performed by the application (or any library it uses) that relies on the default HTTP client will now bypass TLS certificate verification.
# 1. Run the proxy with insecure TLS verification enabled
./oauth2-proxy --ssl-insecure-skip-verify=true --client-id=... --client-secret=... --cookie-secret=... --upstream=...

# 2. Any outgoing HTTPS request made by the process (e.g., to an OIDC discovery endpoint or another service) 
# will now proceed without validating the server's TLS certificate.
# An attacker positioned in the network can intercept this traffic and present a fraudulent certificate.
Trace
graph TD
    subgraph SG0 ["./oauth2-proxy/main.go"]
        main["Main entry point for the application, handles configuration and server initialization."]
    end
    style SG0 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG1 ["./oauth2-proxy/pkg/encryption/cipher.go"]
        NewBase64Cipher["Constructs a base64-wrapped cipher for cookie encryption."]
    end
    style SG1 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG2 ["./oauth2-proxy/pkg/encryption/utils.go"]
        SecretBytes["Converts a secret string into byte format, handling base64 decoding."]
    end
    style SG2 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG3 ["./oauth2-proxy/pkg/ip/realclientip.go"]
        GetRealClientIPParser["Factory for creating a client IP parser based on header keys."]
        GetRealClientIP["Parses the real client IP from HTTP headers."]
        getRemoteIP["Obtains the remote IP from the request connection."]
        GetClientString["Returns a human-readable string for the client IP."]
    end
    style SG3 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG4 ["./oauth2-proxy/pkg/logger/logger.go"]
        New["Creates a new logger instance."]
        Output["Outputs a standard log message using a template."]
        GetFileLineString["GetFileLineString"]
        FormatTimestamp["Formats a timestamp."]
        SetGetClientFunc["Sets the function to retrieve the client IP."]
        Printf["Prints a formatted log message."]
    end
    style SG4 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG5 ["./oauth2-proxy/pkg/requests/requests.go"]
        Request["Performs an HTTP request and parses the JSON response."]
    end
    style SG5 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG6 ["./oauth2-proxy/pkg/sessions/cookie/session_store.go"]
        NewCookieSessionStore["Initializes a cookie-based session store."]
    end
    style SG6 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG7 ["./oauth2-proxy/pkg/sessions/redis/redis_store.go"]
        NewRedisSessionStore["Initializes a Redis-based session store."]
        newRedisCmdable["newRedisCmdable"]
        parseRedisURLs["parseRedisURLs"]
    end
    style SG7 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG8 ["./oauth2-proxy/pkg/sessions/session_store.go"]
        NewSessionStore["Factory function to create a session store implementation."]
    end
    style SG8 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG9 ["./oauth2-proxy/pkg/validation/options.go"]
        Validate{{"Validates application configuration options, initializes security ciphers, and sets up authentication verifiers during startup."}}
        parseProviderInfo["parseProviderInfo"]
        parseSignatureKey["parseSignatureKey"]
        parseJwtIssuers["parseJwtIssuers"]
        newVerifierFromJwtIssuer["newVerifierFromJwtIssuer"]
        validateCookieName["validateCookieName"]
        parseURL["parseURL"]
    end
    style SG9 fill:#2a2a2a,stroke:#444,color:#aaa
    Validate --> GetRealClientIPParser
    Validate --> GetClientString
    Validate --> NewSessionStore
    Validate --> NewBase64Cipher
    Validate --> SecretBytes
    Validate --> SetGetClientFunc
    Validate --> Printf
    Validate --> Request
    Validate --> parseProviderInfo
    Validate --> parseSignatureKey
    Validate --> parseJwtIssuers
    Validate --> newVerifierFromJwtIssuer
    Validate --> validateCookieName
    Validate --> parseURL
    GetClientString --> GetRealClientIP
    GetClientString --> getRemoteIP
    NewSessionStore --> NewRedisSessionStore
    NewSessionStore --> NewCookieSessionStore
    SetGetClientFunc --> SetGetClientFunc
    Printf --> Output
    Request --> Printf
    parseProviderInfo --> New
    parseProviderInfo --> parseURL
    NewRedisSessionStore --> newRedisCmdable
    Output --> GetFileLineString
    Output --> FormatTimestamp
    New --> New
    newRedisCmdable --> parseRedisURLs
    newRedisCmdable --> Printf
    FormatTimestamp --> FormatTimestamp
    main --> Validate
Loading
Fix with AI

Open in Cursor Open in Claude

Fix the following security vulnerability found by Hacktron.

File: pkg/validation/options.go
Lines: 267
Severity: high

Vulnerability: Global Insecure TLS Configuration via http.DefaultClient Modification

Description:
The application modifies the global `http.DefaultClient` when the `--ssl-insecure-skip-verify` flag is enabled. In Go, `http.DefaultClient` is a shared global variable used by many functions (e.g., `http.Get`, `http.Post`). Modifying this variable affects all outgoing HTTPS requests made by the entire application process, not just those intended for the OAuth2 provider. This creates a global risk of Man-in-the-Middle (MitM) attacks for any HTTPS communication performed by the proxy, including potentially sensitive traffic to other services or dependencies.

Proof of Concept:
**Steps to Reproduce**

1. Configure the application to run with the `--ssl-insecure-skip-verify` flag enabled.
2. The application's `Validate` function executes, which overwrites the global `http.DefaultClient` with a new `http.Client` configured with `InsecureSkipVerify: true`.
3. Any subsequent outgoing HTTPS request performed by the application (or any library it uses) that relies on the default HTTP client will now bypass TLS certificate verification.

```bash
# 1. Run the proxy with insecure TLS verification enabled
./oauth2-proxy --ssl-insecure-skip-verify=true --client-id=... --client-secret=... --cookie-secret=... --upstream=...

# 2. Any outgoing HTTPS request made by the process (e.g., to an OIDC discovery endpoint or another service) 
# will now proceed without validating the server's TLS certificate.
# An attacker positioned in the network can intercept this traffic and present a fraudulent certificate.
```

Affected Code:
if o.SSLInsecureSkipVerify {
		// TODO: Accept a certificate bundle.
		insecureTransport := &http.Transport{
			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
		}
		http.DefaultClient = &http.Client{Transport: insecureTransport}
	}

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

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 Flawed Whitelist Domain Suffix Matching

The IsValidRedirect function in oauthproxy.go incorrectly validates redirect URLs against the whitelistDomains configuration. When a domain is configured with a leading dot to allow subdomains (e.g., .example.com), the code uses strings.HasSuffix to verify the redirectHostname against the domainHostname. This check fails to ensure that the matched suffix is preceded by a dot, allowing an attacker to bypass the whitelist by supplying a hostname that merely ends with the whitelisted domain string (e.g., badexample.com will match the whitelisted .example.com). This enables an open redirect vulnerability, allowing attackers to redirect users to arbitrary malicious domains.

Steps to Reproduce
  1. Configure oauth2-proxy with --whitelist-domain=.example.com.
  2. Craft a malicious redirect URL: http://<proxy-host>/oauth2/start?rd=http://badexample.com.
  3. The IsValidRedirect function will parse badexample.com, identify it as a suffix of example.com, and return true, allowing the redirect to proceed to the malicious domain.
Trace
graph TD
    subgraph SG0 ["./oauth2-proxy/oauthproxy.go"]
        ServeHTTP["Main HTTP request handler for routing and authentication flow."]
        SignInPage["SignInPage"]
        GetRedirect["GetRedirect"]
        splitHostPort["splitHostPort"]
        validOptionalPort["validOptionalPort"]
        IsValidRedirect{{"IsValidRedirect"}}
        SignIn["SignIn"]
        SignOut["SignOut"]
        OAuthStart["OAuthStart"]
        OAuthCallback["OAuthCallback"]
        Proxy["Proxy"]
    end
    style SG0 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG1 ["./oauth2-proxy/pkg/logger/logger.go"]
        Output["Outputs a standard log message using a template."]
        GetFileLineString["GetFileLineString"]
        FormatTimestamp["Formats a timestamp."]
        Printf["Prints a formatted log message."]
    end
    style SG1 fill:#2a2a2a,stroke:#444,color:#aaa
    IsValidRedirect --> Printf
    IsValidRedirect --> splitHostPort
    Printf --> Output
    splitHostPort --> validOptionalPort
    Output --> GetFileLineString
    Output --> FormatTimestamp
    FormatTimestamp --> FormatTimestamp
    GetRedirect --> IsValidRedirect
    OAuthCallback --> IsValidRedirect
    SignInPage --> GetRedirect
    SignIn --> GetRedirect
    SignOut --> GetRedirect
    OAuthStart --> GetRedirect
    ServeHTTP --> OAuthCallback
    SignIn --> SignInPage
    Proxy --> SignInPage
    ServeHTTP --> SignIn
    ServeHTTP --> SignOut
    ServeHTTP --> OAuthStart
    SignIn --> OAuthStart
    Proxy --> OAuthStart
    ServeHTTP --> ServeHTTP
    Proxy --> ServeHTTP
    ServeHTTP --> Proxy
Loading
Fix with AI

Open in Cursor Open in Claude

Fix the following security vulnerability found by Hacktron.

File: oauthproxy.go
Severity: medium

Vulnerability: Open Redirect via Flawed Whitelist Domain Suffix Matching

Description:
The `IsValidRedirect` function in `oauthproxy.go` incorrectly validates redirect URLs against the `whitelistDomains` configuration. When a domain is configured with a leading dot to allow subdomains (e.g., `.example.com`), the code uses `strings.HasSuffix` to verify the `redirectHostname` against the `domainHostname`. This check fails to ensure that the matched suffix is preceded by a dot, allowing an attacker to bypass the whitelist by supplying a hostname that merely ends with the whitelisted domain string (e.g., `badexample.com` will match the whitelisted `.example.com`). This enables an open redirect vulnerability, allowing attackers to redirect users to arbitrary malicious domains.

Proof of Concept:
**Steps to Reproduce**

1. Configure `oauth2-proxy` with `--whitelist-domain=.example.com`.
2. Craft a malicious redirect URL: `http://<proxy-host>/oauth2/start?rd=http://badexample.com`.
3. The `IsValidRedirect` function will parse `badexample.com`, identify it as a suffix of `example.com`, and return `true`, allowing the redirect to proceed to the malicious 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 thread pkg/validation/options.go

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: Insecure Cryptographic Initialization Vector Usage in Redis Session Store

The Redis session store implementation uses the ticket.Secret as both the AES encryption key and the Initialization Vector (IV) for the AES-CFB mode. While the ticket.Secret is unique per session, using the secret key as the IV is a cryptographic anti-pattern. In CFB mode, the IV must be unpredictable. Since the IV is derived from the secret key, this violates the requirement for an unpredictable IV. Furthermore, using the same key as the IV results in deterministic encryption for subsequent updates to the same session, which can potentially leak information about the session state if the same data is encrypted multiple times with the same session key.

Trace
graph TD
    subgraph SG0 ["./oauth2-proxy/main.go"]
        main["Main entry point for the application, handles configuration and server initialization."]
    end
    style SG0 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG1 ["./oauth2-proxy/pkg/logger/logger.go"]
        Output["Outputs a standard log message using a template."]
        GetFileLineString["GetFileLineString"]
        FormatTimestamp["Formats a timestamp."]
        Printf["Prints a formatted log message."]
    end
    style SG1 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG2 ["./oauth2-proxy/pkg/sessions/redis/redis_store.go"]
        NewRedisSessionStore{{"Initializes a Redis-based session store."}}
        newRedisCmdable["newRedisCmdable"]
        parseRedisURLs["parseRedisURLs"]
    end
    style SG2 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG3 ["./oauth2-proxy/pkg/sessions/session_store.go"]
        NewSessionStore["Factory function to create a session store implementation."]
    end
    style SG3 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG4 ["./oauth2-proxy/pkg/sessions/session_store_test.go"]
        RunSessionTests["RunSessionTests"]
        session_store_test.go["session_store_test.go"]
    end
    style SG4 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG5 ["./oauth2-proxy/pkg/validation/options.go"]
        Validate["Validates application configuration options, initializes security ciphers, and sets up authentication verifiers during startup."]
    end
    style SG5 fill:#2a2a2a,stroke:#444,color:#aaa
    NewRedisSessionStore --> newRedisCmdable
    newRedisCmdable --> parseRedisURLs
    newRedisCmdable --> Printf
    Printf --> Output
    Output --> GetFileLineString
    Output --> FormatTimestamp
    FormatTimestamp --> FormatTimestamp
    NewSessionStore --> NewRedisSessionStore
    RunSessionTests --> NewSessionStore
    session_store_test.go --> NewSessionStore
    Validate --> NewSessionStore
    session_store_test.go --> RunSessionTests
    main --> Validate
Loading
Fix with AI

Open in Cursor Open in Claude

Fix the following security vulnerability found by Hacktron.

File: pkg/validation/options.go
Severity: medium

Vulnerability: Insecure Cryptographic Initialization Vector Usage in Redis Session Store

Description:
The Redis session store implementation uses the `ticket.Secret` as both the AES encryption key and the Initialization Vector (IV) for the AES-CFB mode. While the `ticket.Secret` is unique per session, using the secret key as the IV is a cryptographic anti-pattern. In CFB mode, the IV must be unpredictable. Since the IV is derived from the secret key, this violates the requirement for an unpredictable IV. Furthermore, using the same key as the IV results in deterministic encryption for subsequent updates to the same session, which can potentially leak information about the session state if the same data is encrypted multiple times with the same session key.

Affected Code:
// Use secret as the IV too, because each entry has it's own key
stream := cipher.NewCFBDecrypter(block, ticket.Secret)
stream.XORKeyStream(resultBytes, resultBytes)

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 pkg/validation/options.go

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: Unbounded Cookie Reconstruction Denial of Service

The loadCookie function in pkg/sessions/cookie/session_store.go reconstructs session cookies by iterating through a sequence of cookies (e.g., _oauth2_proxy_0, _oauth2_proxy_1, etc.) until a cookie is not found. There is no upper bound on the number of cookies processed or the total size of the reconstructed cookie. An attacker can send a large number of crafted cookies in a single request, forcing the application to perform excessive memory allocations and string concatenations, which can lead to a denial-of-service (DoS) condition via memory exhaustion.

Steps to Reproduce
  1. Identify the configured cookie name (default is _oauth2_proxy).
  2. Construct an HTTP request containing a large number of cookies named _oauth2_proxy_0, _oauth2_proxy_1, ..., _oauth2_proxy_N.
  3. Send the request to the proxy.
  4. Observe the memory usage of the proxy process increasing significantly as it attempts to reconstruct the session cookie.
# Generate a large number of cookies and send them in a request
# This example sends 1000 cookies, each 3840 bytes long.
cookies=""
for i in {0..1000}; do
  cookies="${cookies}_oauth2_proxy_${i}=$(head -c 3840 /dev/urandom | base64); "
done
curl -H "Cookie: $cookies" http://<proxy-address>/
Trace
graph TD
    subgraph SG0 ["./oauth2-proxy/main.go"]
        main["Main entry point for the application, handles configuration and server initialization."]
    end
    style SG0 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG1 ["./oauth2-proxy/oauthproxy_test.go"]
        TestClearSplitCookie["TestClearSplitCookie"]
        TestClearSingleCookie["TestClearSingleCookie"]
    end
    style SG1 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG2 ["./oauth2-proxy/pkg/sessions/cookie/session_store.go"]
        NewCookieSessionStore{{"Initializes a cookie-based session store."}}
    end
    style SG2 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG3 ["./oauth2-proxy/pkg/sessions/session_store.go"]
        NewSessionStore["Factory function to create a session store implementation."]
    end
    style SG3 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG4 ["./oauth2-proxy/pkg/sessions/session_store_test.go"]
        RunSessionTests["RunSessionTests"]
        session_store_test.go["session_store_test.go"]
    end
    style SG4 fill:#2a2a2a,stroke:#444,color:#aaa
    subgraph SG5 ["./oauth2-proxy/pkg/validation/options.go"]
        Validate["Validates application configuration options, initializes security ciphers, and sets up authentication verifiers during startup."]
    end
    style SG5 fill:#2a2a2a,stroke:#444,color:#aaa
    NewSessionStore --> NewCookieSessionStore
    TestClearSplitCookie --> NewCookieSessionStore
    TestClearSingleCookie --> NewCookieSessionStore
    RunSessionTests --> NewSessionStore
    session_store_test.go --> NewSessionStore
    Validate --> NewSessionStore
    session_store_test.go --> RunSessionTests
    main --> Validate
Loading
Fix with AI

Open in Cursor Open in Claude

Fix the following security vulnerability found by Hacktron.

File: pkg/validation/options.go
Severity: medium

Vulnerability: Unbounded Cookie Reconstruction Denial of Service

Description:
The `loadCookie` function in `pkg/sessions/cookie/session_store.go` reconstructs session cookies by iterating through a sequence of cookies (e.g., `_oauth2_proxy_0`, `_oauth2_proxy_1`, etc.) until a cookie is not found. There is no upper bound on the number of cookies processed or the total size of the reconstructed cookie. An attacker can send a large number of crafted cookies in a single request, forcing the application to perform excessive memory allocations and string concatenations, which can lead to a denial-of-service (DoS) condition via memory exhaustion.

Proof of Concept:
**Steps to Reproduce**

1. Identify the configured cookie name (default is `_oauth2_proxy`).
2. Construct an HTTP request containing a large number of cookies named `_oauth2_proxy_0`, `_oauth2_proxy_1`, ..., `_oauth2_proxy_N`.
3. Send the request to the proxy.
4. Observe the memory usage of the proxy process increasing significantly as it attempts to reconstruct the session cookie.

```bash
# Generate a large number of cookies and send them in a request
# This example sends 1000 cookies, each 3840 bytes long.
cookies=""
for i in {0..1000}; do
  cookies="${cookies}_oauth2_proxy_${i}=$(head -c 3840 /dev/urandom | base64); "
done
curl -H "Cookie: $cookies" http://<proxy-address>/
```

Affected Code:
func loadCookie(req *http.Request, cookieName string) (*http.Cookie, error) {
	c, err := req.Cookie(cookieName)
	if err == nil {
		return c, nil
	}
	cookies := []*http.Cookie{}
	err = nil
	count := 0
	for err == nil {
		var c *http.Cookie
		c, err = req.Cookie(fmt.Sprintf("%s_%d", cookieName, count))
		if err == nil {
			cookies = append(cookies, c)
			count++
		}
	}
	if len(cookies) == 0 {
		return nil, fmt.Errorf("could not find cookie %s", cookieName)
	}
	return joinCookies(cookies)
}

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

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