[release-11.6.8] Stricter validation for redirect URLs (#113863)

Stricter validation for redirect URLs (#113852)

(cherry picked from commit 3f48a6358f)

pkg/api/login.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
-	"path"
 	"regexp"
 	"strings"
 
@@ -41,8 +40,10 @@ var getViewIndex = func() string {
 	return viewIndex
 }
 
-// Only allow redirects that start with a slash followed by an alphanumerical character, a dash or an underscore.
+var redirectAllowRe = regexp.MustCompile(`^/[a-zA-Z0-9-_./]*$`)
-var redirectRe = regexp.MustCompile(`^/[a-zA-Z0-9-_].*`)
+
+// Do not allow redirect URLs that contain "//" or ".."
+var redirectDenyRe = regexp.MustCompile(`(//|\.\.)`)
 
 var (
 	errAbsoluteRedirectTo  = errors.New("absolute URLs are not allowed for redirect_to cookie value")
@@ -64,26 +65,11 @@ func (hs *HTTPServer) ValidateRedirectTo(redirectTo string) error {
 		return errForbiddenRedirectTo
 	}
 
-	// path should have exactly one leading slash
+	if redirectDenyRe.MatchString(to.Path) {
-	if !strings.HasPrefix(to.Path, "/") {
 		return errForbiddenRedirectTo
 	}
 
-	if strings.HasPrefix(to.Path, "//") {
+	if to.Path != "/" && !redirectAllowRe.MatchString(to.Path) {
-		return errForbiddenRedirectTo
-	}
-
-	if to.Path != "/" && !redirectRe.MatchString(to.Path) {
-		return errForbiddenRedirectTo
-	}
-
-	cleanPath := path.Clean(to.Path)
-	// "." is what path.Clean returns for empty paths
-	if cleanPath == "." {
-		return errForbiddenRedirectTo
-	}
-
-	if cleanPath != "/" && !redirectRe.MatchString(cleanPath) {
 		return errForbiddenRedirectTo
 	}
 

pkg/middleware/org_redirect.go

@@ -3,7 +3,6 @@ package middleware
 import (
 	"fmt"
 	"net/http"
-	"path"
 	"regexp"
 	"strconv"
 
@@ -13,8 +12,10 @@ import (
 	"github.com/grafana/grafana/pkg/web"
 )
 
-// Only allow redirects that start with a slash followed by an alphanumerical character, a dash or an underscore.
+var redirectAllowRe = regexp.MustCompile(`^/?[a-zA-Z0-9-_./]*$`)
-var redirectRe = regexp.MustCompile(`^/?[a-zA-Z0-9-_].*`)
+
+// Do not allow redirect URLs that contain "//" or ".."
+var redirectDenyRe = regexp.MustCompile(`(//|\.\.)`)
 
 // OrgRedirect changes org and redirects users if the
 // querystring `orgId` doesn't match the active org.
@@ -66,9 +67,9 @@ func OrgRedirect(cfg *setting.Cfg, userSvc user.Service) web.Handler {
 }
 
 func validRedirectPath(p string) bool {
-	if p != "" && p != "/" && !redirectRe.MatchString(p) {
+	if redirectDenyRe.MatchString(p) {
 		return false
 	}
-	cleanPath := path.Clean(p)
+
-	return cleanPath == "." || cleanPath == "/" || redirectRe.MatchString(cleanPath)
+	return p == "" || p == "/" || redirectAllowRe.MatchString(p)
 }