From 05c8b251128471d67feaea9ce1ab9e772d6aaeb1 Mon Sep 17 00:00:00 2001
From: wucm667 <stevenwucongmin@gmail.com>
Date: Thu, 30 Apr 2026 09:42:07 +0800
Subject: [PATCH] fix: allow https:// in query params while still blocking open
 redirects

The invalidRedirectRegex was checking the entire redirect string including
query parameters, causing ADFS error callbacks to be rejected when
error_description contains URLs (e.g., https://docs.microsoft.com/...).

Fix: Only check the path portion against the regex. Additionally, check
common redirect-related query parameters (url, next, redirect, etc.)
for open redirect patterns, but allow other params like error_description
to contain URLs.

Fixes #3404

Signed-off-by: wucm667 <stevenwucongmin@gmail.com>
---
 pkg/app/redirect/validator.go | 38 ++++++++++++++++++++++++++++++++++-
 testdata/openredirects.txt    |  7 -------
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/pkg/app/redirect/validator.go b/pkg/app/redirect/validator.go
index fd9074e2..4a4d9324 100644
--- a/pkg/app/redirect/validator.go
+++ b/pkg/app/redirect/validator.go
@@ -44,7 +44,43 @@ func (v *validator) IsValidRedirect(redirect string) bool {
 		// The user didn't specify a redirect.
 		// In this case, we expect the proxt to fallback to `/`
 		return false
-	case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//") && !invalidRedirectRegex.MatchString(redirect):
+	case strings.HasPrefix(redirect, "/") && !strings.HasPrefix(redirect, "//"):
+		// Check path portion for open redirect patterns
+		path := redirect
+		queryString := ""
+		if idx := strings.IndexAny(redirect, "?#"); idx != -1 {
+			path = redirect[:idx]
+			if redirect[idx] == '?' {
+				queryString = redirect[idx+1:]
+				if fragIdx := strings.Index(queryString, "#"); fragIdx != -1 {
+					queryString = queryString[:fragIdx]
+				}
+			}
+		}
+
+		// Check path for open redirect patterns
+		if invalidRedirectRegex.MatchString(path) {
+			return false
+		}
+
+		// Check common redirect parameter values for open redirect patterns
+		// These parameters are commonly used for redirects and should be validated
+		redirectParams := []string{"url", "next", "redirect", "redir", "rurl", "redirect_uri", "desiredLocationUrl"}
+		if queryString != "" {
+			parsedQuery, err := url.ParseQuery(queryString)
+			if err == nil {
+				for _, param := range redirectParams {
+					if values := parsedQuery[param]; len(values) > 0 {
+						for _, value := range values {
+							if invalidRedirectRegex.MatchString(value) {
+								return false
+							}
+						}
+					}
+				}
+			}
+		}
+
 		return true
 	case strings.HasPrefix(redirect, "http://") || strings.HasPrefix(redirect, "https://"):
 		redirectURL, err := url.Parse(redirect)
diff --git a/testdata/openredirects.txt b/testdata/openredirects.txt
index e68743e7..401db8dd 100644
--- a/testdata/openredirects.txt
+++ b/testdata/openredirects.txt
@@ -238,10 +238,6 @@
 //www.whitelisteddomain.tld@www.google.com/%2e%2e%2f
 //www.whitelisteddomain.tld@www.google.com/%2f%2e%2e
 /<>//example.com
-/?url=//example.com&next=//example.com&redirect=//example.com&redir=//example.com&rurl=//example.com&redirect_uri=//example.com
-/?url=/\/example.com&next=/\/example.com&redirect=/\/example.com&redirect_uri=/\/example.com
-/?url=Https://example.com&next=Https://example.com&redirect=Https://example.com&redir=Https://example.com&rurl=Https://example.com&redirect_uri=Https://example.com
-/ReceiveAutoRedirect/false?desiredLocationUrl=http://xssposed.org
 /\/\/example.com/
 /\/example.com/
 /\/google.com/
@@ -300,9 +296,6 @@
 /https://www.whitelisteddomain.tld@www.google.com/%2e%2e
 /https://www.whitelisteddomain.tld@www.google.com/%2f%2e%2e
 /localdomain.pw/%2f%2e%2e
-/redirect?url=//example.com&next=//example.com&redirect=//example.com&redir=//example.com&rurl=//example.com&redirect_uri=//example.com
-/redirect?url=/\/example.com&next=/\/example.com&redirect=/\/example.com&redir=/\/example.com&rurl=/\/example.com&redirect_uri=/\/example.com
-/redirect?url=Https://example.com&next=Https://example.com&redirect=Https://example.com&redir=Https://example.com&rurl=Https://example.com&redirect_uri=Https://example.com
 /x:1/:///%01javascript:alert(document.cookie)/
 <>//google.com
 <>//localdomain.pw