Security: Fixes for CVE-2025-6197 and CVE-2025-6023 (#108280)

fix(redirect): make validation of redirect uri stricter

Co-authored-by: volcanonoodle <114113189+volcanonoodle@users.noreply.github.com>

pkg/api/login.go

@@ -7,6 +7,8 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
+	"path"
+	"regexp"
 	"strings"
 
 	"github.com/grafana/grafana/pkg/api/response"
@@ -39,6 +41,9 @@ var getViewIndex = func() string {
 	return viewIndex
 }
 
+// Only allow redirects that start with an alphanumerical character, a dash or an underscore.
+var redirectRe = regexp.MustCompile(`^/[a-zA-Z0-9-_].*`)
+
 var (
 	errAbsoluteRedirectTo  = errors.New("absolute URLs are not allowed for redirect_to cookie value")
 	errInvalidRedirectTo   = errors.New("invalid redirect_to cookie value")
@@ -68,6 +73,15 @@ func (hs *HTTPServer) ValidateRedirectTo(redirectTo string) error {
 		return errForbiddenRedirectTo
 	}
 
+	cleanPath := path.Clean(to.Path)
+	// "." is what path.Clean returns for empty paths
+	if cleanPath == "." {
+		return errForbiddenRedirectTo
+	}
+	if to.Path != "/" && !redirectRe.MatchString(cleanPath) {
+		return errForbiddenRedirectTo
+	}
+
 	// when using a subUrl, the redirect_to should start with the subUrl (which contains the leading slash), otherwise the redirect
 	// will send the user to the wrong location
 	if hs.Cfg.AppSubURL != "" && !strings.HasPrefix(to.Path, hs.Cfg.AppSubURL+"/") {

pkg/api/login_oauth_test.go

@@ -14,20 +14,16 @@ import (
 	"github.com/grafana/grafana/pkg/services/authn/authntest"
 	"github.com/grafana/grafana/pkg/services/secrets/fakes"
 	"github.com/grafana/grafana/pkg/setting"
+	"github.com/grafana/grafana/pkg/web/webtest"
 )
 
-func setClientWithoutRedirectFollow(t *testing.T) {
+func setClientWithoutRedirectFollow(t *testing.T, s *webtest.Server) {
 	t.Helper()
-	old := http.DefaultClient
-	http.DefaultClient = &http.Client{
+	s.HttpClient = &http.Client{
 		CheckRedirect: func(req *http.Request, via []*http.Request) error {
 			return http.ErrUseLastResponse
 		},
 	}
-
-	t.Cleanup(func() {
-		http.DefaultClient = old
-	})
 }
 
 func TestOAuthLogin_Redirect(t *testing.T) {
@@ -79,7 +75,7 @@ func TestOAuthLogin_Redirect(t *testing.T) {
 			})
 
 			// we need to prevent the http.Client from following redirects
-			setClientWithoutRedirectFollow(t)
+			setClientWithoutRedirectFollow(t, server)
 
 			res, err := server.Send(server.NewGetRequest("/login/generic_oauth"))
 			require.NoError(t, err)
@@ -155,7 +151,7 @@ func TestOAuthLogin_AuthorizationCode(t *testing.T) {
 			})
 
 			// we need to prevent the http.Client from following redirects
-			setClientWithoutRedirectFollow(t)
+			setClientWithoutRedirectFollow(t, server)
 
 			res, err := server.Send(server.NewGetRequest("/login/generic_oauth?code=code"))
 			require.NoError(t, err)
@@ -199,7 +195,7 @@ func TestOAuthLogin_Error(t *testing.T) {
 		hs.SecretsService = fakes.NewFakeSecretsService()
 	})
 
-	setClientWithoutRedirectFollow(t)
+	setClientWithoutRedirectFollow(t, server)
 
 	res, err := server.Send(server.NewGetRequest("/login/azuread?error=someerror"))
 	require.NoError(t, err)

pkg/api/user_token_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"net/url"
 	"testing"
 	"time"
 
@@ -20,6 +21,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/user"
 	"github.com/grafana/grafana/pkg/services/user/usertest"
 	"github.com/grafana/grafana/pkg/setting"
+	"github.com/grafana/grafana/pkg/web/webtest"
 )
 
 func TestUserTokenAPIEndpoint(t *testing.T) {
@@ -150,6 +152,95 @@ func TestUserTokenAPIEndpoint(t *testing.T) {
 	})
 }
 
+func TestHTTPServer_RotateUserAuthTokenRedirect(t *testing.T) {
+	redirectTestCases := []struct {
+		name        string
+		redirectUrl string
+		expectedUrl string
+	}{
+		// Valid redirects should be preserved
+		{"valid root path", "/", "/"},
+		{"valid simple path", "/hello", "/hello"},
+		{"valid single char path", "/a", "/a"},
+		{"valid nested path", "/asd/hello", "/asd/hello"},
+
+		// Invalid redirects should be converted to root
+		{"backslash domain", `/\grafana.com`, "/"},
+		{"traversal backslash domain", `/a/../\grafana.com`, "/"},
+		{"double slash", "//grafana", "/"},
+		{"missing initial slash", "missingInitialSlash", "/"},
+		{"parent directory", "/../", "/"},
+	}
+
+	sessionTestCases := []struct {
+		name                      string
+		useSessionStorageRedirect bool
+	}{
+		{"when useSessionStorageRedirect is enabled", true},
+		{"when useSessionStorageRedirect is disabled", false},
+	}
+
+	for _, sessionCase := range sessionTestCases {
+		t.Run(sessionCase.name, func(t *testing.T) {
+			for _, redirectCase := range redirectTestCases {
+				t.Run(redirectCase.name, func(t *testing.T) {
+					server := SetupAPITestServer(t, func(hs *HTTPServer) {
+						cfg := setting.NewCfg()
+						cfg.LoginCookieName = "grafana_session"
+						cfg.LoginMaxLifetime = 10 * time.Hour
+						hs.Cfg = cfg
+						hs.log = log.New()
+						hs.AuthTokenService = &authtest.FakeUserAuthTokenService{
+							RotateTokenProvider: func(ctx context.Context, cmd auth.RotateCommand) (*auth.UserToken, error) {
+								return &auth.UserToken{UnhashedToken: "new"}, nil
+							},
+						}
+					})
+
+					redirectToQuery := url.QueryEscape(redirectCase.redirectUrl)
+					urlString := "/user/auth-tokens/rotate"
+
+					if sessionCase.useSessionStorageRedirect {
+						urlString = urlString + "?redirectTo=" + redirectToQuery
+					}
+
+					req := server.NewGetRequest(urlString)
+					req.AddCookie(&http.Cookie{Name: "grafana_session", Value: "123", Path: "/"})
+
+					if sessionCase.useSessionStorageRedirect {
+						req = webtest.RequestWithWebContext(req, &contextmodel.ReqContext{UseSessionStorageRedirect: true})
+					} else {
+						req.AddCookie(&http.Cookie{Name: "redirect_to", Value: redirectToQuery, Path: "/"})
+					}
+
+					var redirectStatusCode int
+					var redirectLocation string
+
+					server.HttpClient.CheckRedirect = func(req *http.Request, via []*http.Request) error {
+						if len(via) > 1 {
+							// Stop after first redirect
+							return http.ErrUseLastResponse
+						}
+
+						if req.Response == nil {
+							return nil
+						}
+						redirectStatusCode = req.Response.StatusCode
+						redirectLocation = req.Response.Header.Get("Location")
+						return nil
+					}
+					res, err := server.Send(req)
+					require.NoError(t, err)
+					assert.Equal(t, 302, redirectStatusCode)
+					assert.Equal(t, redirectCase.expectedUrl, redirectLocation)
+
+					require.NoError(t, res.Body.Close())
+				})
+			}
+		})
+	}
+}
+
 func TestHTTPServer_RotateUserAuthToken(t *testing.T) {
 	type testCase struct {
 		desc                 string