Forbid more redirect patterns (#110337)

pkg/middleware/org_redirect.go

@@ -13,7 +13,7 @@ import (
 	"github.com/grafana/grafana/pkg/web"
 )
 
-// Only allow redirects that start with an alphanumerical character, a dash or an underscore.
+// 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-_].*`)
 
 // OrgRedirect changes org and redirects users if the
@@ -66,6 +66,9 @@ func OrgRedirect(cfg *setting.Cfg, userSvc user.Service) web.Handler {
 }
 
 func validRedirectPath(p string) bool {
+	if p != "" && p != "/" && !redirectRe.MatchString(p) {
+		return false
+	}
 	cleanPath := path.Clean(p)
 	return cleanPath == "." || cleanPath == "/" || redirectRe.MatchString(cleanPath)
 }

pkg/middleware/org_redirect_test.go

@@ -72,13 +72,18 @@ func TestOrgRedirectMiddleware(t *testing.T) {
 	})
 
 	middlewareScenario(t, "when redirecting to an invalid path", func(t *testing.T, sc *scenarioContext) {
-		sc.withIdentity(&authn.Identity{})
+		testPaths := []string{
+			url.QueryEscape(`/\example.com`),
+			`/%2fexample.com`,
+		}
+		for _, path := range testPaths {
+			sc.withIdentity(&authn.Identity{})
 
-		path := url.QueryEscape(`/\example.com`)
-		sc.m.Get(url.QueryEscape(path), sc.defaultHandler)
-		sc.fakeReq("GET", fmt.Sprintf("%s?orgId=3", path)).exec()
+			sc.m.Get(url.QueryEscape(path), sc.defaultHandler)
+			sc.fakeReq("GET", fmt.Sprintf("%s?orgId=3", path)).exec()
 
-		require.Equal(t, 404, sc.resp.Code)
+			require.Equal(t, 404, sc.resp.Code, "path: %s", path)
+		}
 	})
 
 	middlewareScenario(t, "works correctly when grafana is served under a subpath", func(t *testing.T, sc *scenarioContext) {