Authorization bypass / Information disclosure

Proof-of-concept (pre-fix vulnerability, versions <=12.3.x): Prerequisites: - Grafana instance (<= 12.3.x) with a library element named, e.g., "Library Panel Name". - A user account without the library.panels:read permission but with an active session (e.g., username: noperms, password: noperms). - curl or any HTTP client. Steps: 1) Authenticate as a user without the library.panels:read permission: curl -u noperms:noperms -s -i -H "Accept: application/json" https://grafana.example/api/library-elements/name/Library%20Panel%20Name 2) Observe the response. In vulnerable versions, this would return HTTP 200 with the library element details (or at least leak the element name) regardless of the missing permission. Expected (pre-fix): - HTTP/1.1 200 OK - JSON body containing the library element details (name, id, etc.). Patched behavior (post-fix in 12.4.0+): - Same request should yield HTTP 403 Forbidden due to missing library.panels:read permission. Notes: - The repository test TestIntegrationLibraryElementNameRouteRequiresReadPermission demonstrates that a user with no library.panels:read permission is forbidden for GET /api/library-elements/name/:name, which aligns with the intended fix.

diff --git a/pkg/services/libraryelements/api.go b/pkg/services/libraryelements/api.go index bc82ab47d961a..1e99b98a17c95 100644 --- a/pkg/services/libraryelements/api.go +++ b/pkg/services/libraryelements/api.go @@ -44,7 +44,7 @@ func (l *LibraryElementService) registerAPIEndpoints() { entities.Get("/", authorize(ac.EvalPermission(ActionLibraryPanelsRead)), routing.Wrap(l.getAllHandler)) // TODO: add wrapper for k8s - requires search entities.Get("/:uid", authorize(ac.EvalPermission(ActionLibraryPanelsRead)), routing.Wrap(l.getHandler)) entities.Get("/:uid/connections/", authorize(ac.EvalPermission(ActionLibraryPanelsRead, uidScope)), routing.Wrap(l.getConnectionsHandler)) - entities.Get("/name/:name", routing.Wrap(l.getByNameHandler)) // TODO: add wrapper for k8s - requires search + entities.Get("/name/:name", authorize(ac.EvalPermission(ActionLibraryPanelsRead)), routing.Wrap(l.getByNameHandler)) // TODO: add wrapper for k8s - requires search entities.Patch("/:uid", authorize(ac.EvalPermission(ActionLibraryPanelsWrite, uidScope)), routing.Wrap(l.patchHandler)) // TODO: add wrapper for k8s }) } diff --git a/pkg/services/libraryelements/libraryelements_permissions_test.go b/pkg/services/libraryelements/libraryelements_permissions_test.go index eea98378ba7c5..bd1aa9669764e 100644 --- a/pkg/services/libraryelements/libraryelements_permissions_test.go +++ b/pkg/services/libraryelements/libraryelements_permissions_test.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "net/url" "testing" "github.com/stretchr/testify/require" @@ -256,6 +257,51 @@ func TestIntegrationLibraryElementGranularPermissions(t *testing.T) { }) } +// TestIntegrationLibraryElementNameRouteRequiresReadPermission guards the route-level RBAC added +// to GET /api/library-elements/name/:name. The unscoped library.panels:read check rejects a user +// who lacks the action entirely (403) while leaving a permitted user unaffected (200). Folder- +// scoped denial is a different code path (handled by the per-element filter) covered elsewhere. +func TestIntegrationLibraryElementNameRouteRequiresReadPermission(t *testing.T) { + testutil.SkipIntegrationTestInShortMode(t) + + dir, path := testinfra.CreateGrafDir(t, testinfra.GrafanaOpts{}) + grafanaListedAddr, env := testinfra.StartGrafanaEnv(t, dir, path) + cfgProvider, err := configprovider.ProvideService(env.Cfg) + require.NoError(t, err) + quotaService := quotaimpl.ProvideService(context.Background(), env.SQLStore, cfgProvider) + orgService, err := orgimpl.ProvideService(env.SQLStore, env.Cfg, quotaService) + require.NoError(t, err) + + sharedOrg, err := orgService.CreateWithMember(context.Background(), &org.CreateOrgCommand{Name: "test org"}) + require.NoError(t, err) + + createUserInOrg(t, env.SQLStore, env.Cfg, user.CreateUserCommand{ + DefaultOrgRole: string(org.RoleAdmin), + Password: "admin2", + Login: "admin2", + OrgID: sharedOrg.ID, + }) + // A user with no basic role has none of the fixed roles, so it lacks + // library.panels:read entirely — which is what trips the route middleware. + createUserInOrg(t, env.SQLStore, env.Cfg, user.CreateUserCommand{ + DefaultOrgRole: string(org.RoleNone), + Password: "noperms", + Login: "noperms", + OrgID: sharedOrg.ID, + }) + + createLibraryElement(t, grafanaListedAddr, "admin2", "admin2", "", http.StatusOK) + const panelName = "Library Panel Name" // the name createLibraryElement assigns + + t.Run("user with library.panels:read can read by name", func(t *testing.T) { + getLibraryElementByName(t, grafanaListedAddr, "admin2", "admin2", panelName, http.StatusOK) + }) + + t.Run("user without library.panels:read is forbidden", func(t *testing.T) { + getLibraryElementByName(t, grafanaListedAddr, "noperms", "noperms", panelName, http.StatusForbidden) + }) +} + /* Helper functions */ @@ -304,6 +350,10 @@ func getLibraryElement(t *testing.T, grafanaListedAddr, user, password, uid stri makeHTTPRequest(t, "GET", fmt.Sprintf("http://%s:%s@%s/api/library-elements/%s", user, password, grafanaListedAddr, uid), nil, expectedStatus) } +func getLibraryElementByName(t *testing.T, grafanaListedAddr, user, password, name string, expectedStatus int) { + makeHTTPRequest(t, "GET", fmt.Sprintf("http://%s:%s@%s/api/library-elements/name/%s", user, password, grafanaListedAddr, url.PathEscape(name)), nil, expectedStatus) +} + func getAllLibraryElements(t *testing.T, grafanaListedAddr, user, password string, expectedStatus int, expectedLength int) { resp := makeHTTPRequest(t, "GET", fmt.Sprintf("http://%s:%s@%s/api/library-elements", user, password, grafanaListedAddr), nil, expectedStatus) if expectedStatus == http.StatusOK {