fix: use X-Forwarded-Uri if it exists for pathRegex match - #5
fix: use X-Forwarded-Uri if it exists for pathRegex match#5deb-hacktron wants to merge 2 commits into
Conversation
the functions `isApiPath` and `isAllowedPath` use the `req.URL.Path` property which leads to faulty behavior when behind a reverse proxy. The correct path can be inferred from the `X-Forwarded-Uri` header by making use of the already provided `requestutil.GetRequestURI` function.
|
|
||
| func isAllowedPath(req *http.Request, route allowedRoute) bool { | ||
| matches := route.pathRegex.MatchString(req.URL.Path) | ||
| matches := route.pathRegex.MatchString(requestutil.GetRequestURI(req)) |
There was a problem hiding this comment.
🟡 Medium: Authentication Bypass via Path Normalization Inconsistency
The isAllowedPath function uses requestutil.GetRequestURI(req) to determine the path for allowlist matching. GetRequestURI returns the raw, unnormalized request URI (e.g., via req.URL.RequestURI()). If the proxy is configured with an allowlist regex like ^/public/.*, an attacker can send a request with path traversal sequences, such as /public/../private. The proxy's regex will match the unnormalized path (/public/../private), allowing the request to bypass authentication. When the proxy forwards this request to the upstream server, the upstream server will likely normalize the path to /private, granting the attacker unauthorized access to protected resources.
Steps to Reproduce
- Configure OAuth2-Proxy with an allowed route regex, e.g.,
--skip-auth-regex=^/public/.*. - Send a request to the proxy with a path traversal sequence:
curl -v "http://<proxy-address>/public/../private/secret". - The proxy evaluates the unnormalized URI (
/public/../private/secret) against the regex^/public/.*, which matches, and bypasses authentication. - The proxy forwards the request to the upstream server.
- The upstream server normalizes the path to
/private/secretand serves the protected resource.
Trace
graph TD
subgraph SG0 ["./oauth2-proxy/oauthproxy.go"]
IsAllowedRequest["IsAllowedRequest"]
isAllowedPath{{"Checks if a request path matches the allowlist for bypassing authentication."}}
isAllowedRoute["isAllowedRoute"]
UserInfo["UserInfo"]
AuthOnly["AuthOnly"]
Proxy["Proxy"]
getAuthenticatedSession["getAuthenticatedSession"]
end
style SG0 fill:#2a2a2a,stroke:#444,color:#aaa
isAllowedRoute --> isAllowedPath
IsAllowedRequest --> isAllowedRoute
getAuthenticatedSession --> IsAllowedRequest
UserInfo --> getAuthenticatedSession
AuthOnly --> getAuthenticatedSession
Proxy --> getAuthenticatedSession
Fix with AI
Fix the following security vulnerability found by Hacktron.
File: oauthproxy.go
Lines: 557
Severity: medium
Vulnerability: Authentication Bypass via Path Normalization Inconsistency
Description:
The `isAllowedPath` function uses `requestutil.GetRequestURI(req)` to determine the path for allowlist matching. `GetRequestURI` returns the raw, unnormalized request URI (e.g., via `req.URL.RequestURI()`). If the proxy is configured with an allowlist regex like `^/public/.*`, an attacker can send a request with path traversal sequences, such as `/public/../private`. The proxy's regex will match the unnormalized path (`/public/../private`), allowing the request to bypass authentication. When the proxy forwards this request to the upstream server, the upstream server will likely normalize the path to `/private`, granting the attacker unauthorized access to protected resources.
Proof of Concept:
**Steps to Reproduce**
1. Configure OAuth2-Proxy with an allowed route regex, e.g., `--skip-auth-regex=^/public/.*`.
2. Send a request to the proxy with a path traversal sequence: `curl -v "http://<proxy-address>/public/../private/secret"`.
3. The proxy evaluates the unnormalized URI (`/public/../private/secret`) against the regex `^/public/.*`, which matches, and bypasses authentication.
4. The proxy forwards the request to the upstream server.
5. The upstream server normalizes the path to `/private/secret` and serves the protected resource.
Affected Code:
func isAllowedPath(req *http.Request, route allowedRoute) bool {
matches := route.pathRegex.MatchString(requestutil.GetRequestURI(req))
if route.negate {
return !matches
}
return matches
}
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.
| matches := route.pathRegex.MatchString(requestutil.GetRequestURI(req)) | ||
|
|
||
| if route.negate { | ||
| return !matches |
There was a problem hiding this comment.
🟡 Medium: Lack of Rate Limiting on Basic Authentication
The ManualSignIn function in oauthproxy.go processes basic authentication requests using htpasswd files without implementing any server-side rate limiting or account lockout mechanisms. This allows an attacker to perform high-speed, automated brute-force attacks against local user accounts. While administrators are often expected to implement rate limiting at the edge (e.g., via Nginx), the lack of built-in protection makes the application vulnerable when deployed in environments without such external controls.
Steps to Reproduce
# Example of a simple brute-force loop against the sign_in endpoint
while read -r password; do
curl -s -o /dev/null -w "%{http_code}" -X POST -d "username=admin&password=$password" http://<oauth2-proxy-host>/oauth2/sign_in
echo " - Tried password: $password"
done < passwords.txtTrace
graph TD
subgraph SG0 ["./oauth2-proxy/oauthproxy.go"]
oauthproxy.go{{"Top-level initialization of the OAuthProxy, including configuration parsing, middleware chain construction, and server setup."}}
end
style SG0 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG1 ["./oauth2-proxy/pkg/logger/logger.go"]
New["Creates a new Logger instance with default configuration."]
end
style SG1 fill:#2a2a2a,stroke:#444,color:#aaa
oauthproxy.go --> New
New --> New
Fix with AI
Fix the following security vulnerability found by Hacktron.
File: oauthproxy.go
Lines: 557-578
Severity: medium
Vulnerability: Lack of Rate Limiting on Basic Authentication
Description:
The `ManualSignIn` function in `oauthproxy.go` processes basic authentication requests using `htpasswd` files without implementing any server-side rate limiting or account lockout mechanisms. This allows an attacker to perform high-speed, automated brute-force attacks against local user accounts. While administrators are often expected to implement rate limiting at the edge (e.g., via Nginx), the lack of built-in protection makes the application vulnerable when deployed in environments without such external controls.
Proof of Concept:
```bash
# Example of a simple brute-force loop against the sign_in endpoint
while read -r password; do
curl -s -o /dev/null -w "%{http_code}" -X POST -d "username=admin&password=$password" http://<oauth2-proxy-host>/oauth2/sign_in
echo " - Tried password: $password"
done < passwords.txt
```
Affected Code:
func (p *OAuthProxy) ManualSignIn(req *http.Request) (string, bool, int) {
if req.Method != "POST" || p.basicAuthValidator == nil {
return "", false, http.StatusOK
}
user := req.FormValue("username")
passwd := req.FormValue("password")
if user == "" {
return "", false, http.StatusBadRequest
}
// check auth
if p.basicAuthValidator.Validate(user, passwd) {
logger.PrintAuthf(user, req, logger.AuthSuccess, "Authenticated via HtpasswdFile")
return user, true, http.StatusOK
}
logger.PrintAuthf(user, req, logger.AuthFailure, "Invalid authentication via HtpasswdFile")
return "", false, http.StatusUnauthorized
}
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.
|
Refreshing scan with a new PR. |
Internal demo PR. Dummy-authored recreation of upstream oauth2-proxy PR #2192 for review testing.