GHSA-P64J-F4X9-WQ66: Root-Cause Fix for OAuth Redirect URI Path Validation

Vulnerability

The vulnerable logic accepted OAuth redirect URIs when only the scheme and host matched an allowlisted URL. In internal/service/auth/auth.go, the prior check effectively treated any path on the same origin as trusted, which is inconsistent with the advisory's description of redirect URI path truncation and enables attacker-controlled same-origin paths to be embedded into the OAuth flow. That creates a code-delivery primitive: once the authorization server redirects with ?code=..., the code can be exposed through same-origin pages that leak via Referer, third-party analytics, or chained open redirects, leading to authorization code theft and account takeover as described in the GitHub Security Advisory and the referenced report here.

if strings.EqualFold(redirectURL.Scheme, allowURL.Scheme) &&
			strings.EqualFold(redirectURL.Host, allowURL.Host) {

A second issue was timing: invalid redirect URIs could enter the signed state before being rejected later during callback handling. That expands the attack surface and complicates reasoning about trust boundaries in the authorization flow. The commit linked in the patch explicitly calls this out and moves validation ahead of state JWT issuance.

Patch

The patch makes two material changes in internal/service/auth/auth.go. First, it validates redirectURI before issuing the state JWT, failing fast if parsing or policy validation fails. Second, it changes the allowlist comparison from scheme+host to exact scheme+host+path matching, while intentionally excluding query and fragment from the equality check so the server can append ?code=... after validation.

if redirectURI != "" {
	if _, err := authService.parseAndValidateClientRedirect(redirectURI); err != nil {
		return "", err
	}
}

redirectNorm := strings.ToLower(redirectURL.Scheme) + "://" +
	strings.ToLower(redirectURL.Host) +
	redirectURL.Path
allowNorm := strings.ToLower(allowURL.Scheme) + "://" +
	strings.ToLower(allowURL.Host) +
	allowURL.Path
if redirectNorm == allowNorm {

The added regression test in internal/service/auth/oauth_service_test.go verifies that a same-scheme, same-host redirect with a non-allowlisted path is rejected. That test is directly aligned with the reported exploit condition.

Source references: commit a7e8b8e, GHSA-P64J-F4X9-WQ66 advisory, CVE Reports entry.

Review

Pros

• The fix addresses the actual trust decision that was too weak: redirect URI acceptance now depends on path equality in addition to scheme and host, which closes the same-origin arbitrary-path bypass described in the advisory.
• Validation is moved earlier, before state JWT issuance. This is a sound architectural improvement because it prevents untrusted redirect values from being serialized into state and deferred to callback-time rejection.
• The implementation intent is documented inline with security rationale, including why query and fragment are excluded from comparison.
• A focused regression test covers the critical negative case: same origin but mismatched path must be denied.

Cons

• The visible test coverage is narrow. The patch snippet shows only one denial test; it does not demonstrate positive coverage for exact-match acceptance or edge cases such as default ports, percent-encoding, trailing slashes, empty paths, or path normalization semantics.
• The comparison is string-based after lowercasing scheme and host, with path compared as-is. That may be correct for this application, but it leaves behavior dependent on URL parsing and canonicalization details. For example, whether /auth and /auth/ should be distinct is a policy choice not exercised by the shown tests.
• The duplicated early-validation block in the patch snippet suggests either repeated insertion in the diff excerpt or a potential maintainability issue if duplication exists in the actual code. If present in the source, it should be deduplicated.

Verdict

Root-cause.

This patch fixes the core authorization flaw rather than merely filtering a known exploit string. The vulnerability existed because the redirect URI trust policy was under-specified and accepted any path on a trusted origin; the patch tightens that policy to exact scheme+host+path matching and enforces it before state issuance. Based on the provided diff and test, the remediation is technically aligned with OAuth redirect URI validation requirements and with the exploit narrative in GHSA-P64J-F4X9-WQ66. Follow-up hardening should expand tests around normalization and path edge cases, but the security control itself appears to address the root cause documented in the fixing commit.