Authorization bypass / Privilege escalation

Vulnerability reproduction (pre-fix behavior): - Prereqs: A Grafana instance with library elements enabled. A user account (e.g., editor-user) that has edit rights on a library element but does not have create/write permissions on all folders. Two folders exist: FolderA (accessible for create) and FolderB (not accessible for create). - Action: As editor-user, attempt to relocate a library element from its current folder (FolderA) into FolderB by PATCHing the element with a payload that sets the destination folder UID to FolderB. - Expected (pre-fix): The API call would be accepted and the element would be relocated, because the route guard only checked element-level permissions (library.panels:write) and did not enforce destination-folder permissions. Post-fix behavior (as implemented in this commit): - The API call will check destination-folder permissions using Evaluate(ActionLibraryPanelsCreate, folderUID) on the target folder. If the user lacks create permissions for FolderB, the relocation is rejected with a forbidden/insufficient-permissions error. Proof-of-concept commands (adjust host, credentials, and IDs to your environment): - Prerequisites: Obtain a session token or use basic auth. Set the following placeholders accordingly: BASE_URL=https://grafana.example.com ELEMENT_UID=library-element-uid-to-move DEST_FOLDER_UID=destination-folder-uid (FolderB's UID) USERNAME=editor-user PASSWORD=editor-password - Step 1: Attempt to move the library element to a folder the user cannot access (post-fix would fail): curl -sS -u "$USERNAME:$PASSWORD" -X PATCH \ -H "Content-Type: application/json" \ -d '{"folderUid":"'$DEST_FOLDER_UID'"}' \ "$BASE_URL/api/library/elements/$ELEMENT_UID" -i - Expected result on vulnerable version (pre-fix): HTTP 200 and relocation completed. - Expected result on fixed version: HTTP 403 Forbidden (or 400 with InsufficientPermissions error as per Grafana implementation). Notes: - The patch also defaults empty destination UIDs to the general folder (GeneralFolderUID) to ensure proper scope resolution. - The test suite added with the patch exercises the scenario by granting read-only on the destination folder and expecting a Forbidden response when trying to relocate, validating the new guard.

diff --git a/pkg/services/libraryelements/database.go b/pkg/services/libraryelements/database.go index 9eea2d0a2419a..e10725afc4eea 100644 --- a/pkg/services/libraryelements/database.go +++ b/pkg/services/libraryelements/database.go @@ -616,6 +616,30 @@ func (l *LibraryElementService) PatchLibraryElement(c context.Context, signedInU if f.ManagedBy == utils.ManagerKindRepo { return model.LibraryElementDTO{}, model.ErrLibraryElementProvisionedFolder } + + // The destination folder must allow the caller to create library + // panels there. The route-level authorize guard only checks + // library.panels:write on the element itself, so without this check + // a caller with edit rights on the element could relocate it into + // any folder, including ones they cannot see or write to. + // + // Empty UID is normalized to the "general" sentinel so the scope + // resolves to fixed:folders.general — without this, GetResourceScopeUID("") + // produces "folders:uid:" which never matches a granted permission. + destFolderUID := *cmd.FolderUID + if destFolderUID == "" { + destFolderUID = ac.GeneralFolderUID + } + allowed, err := l.AccessControl.Evaluate(c, signedInUser, + ac.EvalPermission(ActionLibraryPanelsCreate, + folder.ScopeFoldersProvider.GetResourceScopeUID(destFolderUID))) + if err != nil { + return model.LibraryElementDTO{}, err + } + if !allowed { + return model.LibraryElementDTO{}, fmt.Errorf("%w: folder UID '%s'", + model.ErrLibraryElementInsufficientPermissions, destFolderUID) + } } err := l.SQLStore.WithTransactionalDbSession(c, func(session *db.Session) error { diff --git a/pkg/services/libraryelements/libraryelements_permissions_test.go b/pkg/services/libraryelements/libraryelements_permissions_test.go index ea06bbd086977..eea98378ba7c5 100644 --- a/pkg/services/libraryelements/libraryelements_permissions_test.go +++ b/pkg/services/libraryelements/libraryelements_permissions_test.go @@ -15,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/configprovider" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess" "github.com/grafana/grafana/pkg/services/libraryelements/model" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/org/orgimpl" @@ -167,12 +168,17 @@ func TestIntegrationLibraryElementGranularPermissions(t *testing.T) { folder1UID := createTestFolder(t, grafanaListedAddr) folder2UID := createTestFolder(t, grafanaListedAddr) folder3UID := createTestFolder(t, grafanaListedAddr) + folder4UID := createTestFolder(t, grafanaListedAddr) // viewer only has access to folder 1 & 3 - grantFolderPermissions(t, grafanaListedAddr, "granular-viewer", "granular-viewer", folder1UID, userID) - grantFolderPermissions(t, grafanaListedAddr, "granular-viewer", "granular-viewer", folder3UID, userID) + grantFolderPermission(t, grafanaListedAddr, folder1UID, userID, dashboardaccess.PERMISSION_EDIT) + grantFolderPermission(t, grafanaListedAddr, folder3UID, userID, dashboardaccess.PERMISSION_EDIT) // revoke view access to folder2 revokeFolderPermissions(t, grafanaListedAddr, folder2UID, userID) + // read-only access to folder4: the viewer can see it but cannot create + // library panels in it — exercises the destination-folder permission + // check in PatchLibraryElement. + grantFolderPermission(t, grafanaListedAddr, folder4UID, userID, dashboardaccess.PERMISSION_VIEW) uid := "" t.Run("granular createpermissions", func(t *testing.T) { @@ -182,6 +188,11 @@ func TestIntegrationLibraryElementGranularPermissions(t *testing.T) { }) t.Run("When viewer doesn't have read access to folder2, they cannot create library element in folder2", func(t *testing.T) { + // The folder is hidden from the caller by the unified storage + // access check, so folderService.Get returns ErrFolderNotFound + // and the legacy create handler falls back to BadRequest. This + // matches the historical "you can't see the folder" semantics + // (a 4xx that doesn't disclose existence). createLibraryElement(t, grafanaListedAddr, "granular-viewer", "granular-viewer", folder2UID, http.StatusBadRequest) }) @@ -198,6 +209,15 @@ func TestIntegrationLibraryElementGranularPermissions(t *testing.T) { t.Run("When viewer doesn't have read access to folder2, they cannot move library element to folder2", func(t *testing.T) { patchLibraryElement(t, grafanaListedAddr, "granular-viewer", "granular-viewer", uid, folder2UID, http.StatusForbidden) }) + + t.Run("When viewer has read-only access to folder4, they cannot move library element to folder4", func(t *testing.T) { + // Exercises the destination-folder permission check added to + // PatchLibraryElement: folder4 is visible to the caller (so + // folderService.Get succeeds), but library.panels:create is denied + // on it. Without this check, an editor with library.panels:write + // on the element could relocate it into any folder they can see. + patchLibraryElement(t, grafanaListedAddr, "granular-viewer", "granular-viewer", uid, folder4UID, http.StatusForbidden) + }) }) inGeneralFolder := createLibraryElement(t, grafanaListedAddr, "admin2", "admin", "", http.StatusOK) @@ -209,6 +229,10 @@ func TestIntegrationLibraryElementGranularPermissions(t *testing.T) { }) t.Run("When viewer doesn't have read access to folder2, they cannot get library element from folder2", func(t *testing.T) { + // The unified storage access check hides folder2 from the caller + // entirely; folderService.Get returns ErrFolderNotFound, which + // the legacy get handler maps to 404 (NotFound) — k8s-style + // "you don't see this resource" rather than 403. getLibraryElement(t, grafanaListedAddr, "granular-viewer", "granular-viewer", inFolder2, http.StatusNotFound) }) @@ -310,12 +334,12 @@ func createTestFolder(t *testing.T, grafanaListedAddr string) string { return folder.UID } -func grantFolderPermissions(t *testing.T, grafanaListedAddr, user, password, folderUID string, userID int64) { +func grantFolderPermission(t *testing.T, grafanaListedAddr, folderUID string, userID int64, permission dashboardaccess.PermissionType) { permissionRequest := map[string]interface{}{ "items": []map[string]interface{}{ { "userId": userID, - "permission": 2, // edit permission + "permission": int(permission), }, }, }