Stricter validation for redirect URLs (#113852)

This commit is contained in:
xavi
2025-11-13 18:04:20 +01:00
committed by GitHub
parent c1485ecf5f
commit 3f48a6358f
2 changed files with 13 additions and 26 deletions

View File

@@ -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 redirectRe = regexp.MustCompile(`^/[a-zA-Z0-9-_].*`)
var redirectAllowRe = 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 !strings.HasPrefix(to.Path, "/") {
if redirectDenyRe.MatchString(to.Path) {
return errForbiddenRedirectTo
}
if strings.HasPrefix(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) {
if to.Path != "/" && !redirectAllowRe.MatchString(to.Path) {
return errForbiddenRedirectTo
}

View File

@@ -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 redirectRe = regexp.MustCompile(`^/?[a-zA-Z0-9-_].*`)
var redirectAllowRe = 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)
}