Authorization bypass / Information Disclosure

PoC (conceptual, applicable to vulnerable versions prior to the fix): 1) Obtain or assume an identity with non-user type (e.g., AccessPolicy) that can authenticate to Grafana. - This could be a service-account or integration identity used by internal tooling. 2) Call the API endpoint that lists preferences for a given namespace/org without a valid user context. - Endpoint and exact path depend on Grafana deployment, but it generally corresponds to the ListPreferences API in the legacy preferences registry. - Example (illustrative HTTP request; adapt to your Grafana instance): curl -sS -H "Authorization: Bearer <ACCESS_POLICY_TOKEN>" \ https://grafana.example/api/registry/preferences?ns=grafana 3) Expected vulnerable behavior (pre-fix): the response would return a complete list of preferences across users and teams in the org, e.g. a JSON array containing entries like: [ {"org_id": 1, "user_uid": "alice", "team_uid": null, "prefs": {"theme": "dark"}}, {"org_id": 1, "user_uid": "bob", "team_uid": null, "prefs": {"theme": "light"}}, {"org_id": 1, "user_uid": null, "team_uid": "team-a", "prefs": {"homeDash": "dashboard/1"}}, ... ] This exposes per-user and per-team preferences to non-user identities. 4) Impact: attacker could enumerate configuration data tied to users/teams, facilitating information disclosure and potential abuse of user-specific settings. Fix behavior (post-fix) to contrast with PoC: The API now rejects non-user identities with a clear error (e.g., "only users may list preferences"), and requires an explicit All flag to list all preferences if needed. If you test against the fixed version, the curl request above should fail with a 403 or the explicit error message instead of returning the data.

diff --git a/pkg/registry/apis/preferences/legacy/preferences.go b/pkg/registry/apis/preferences/legacy/preferences.go index 5287f251f0702..e407dddfe3dcd 100644 --- a/pkg/registry/apis/preferences/legacy/preferences.go +++ b/pkg/registry/apis/preferences/legacy/preferences.go @@ -78,8 +78,8 @@ func (s *preferenceStorage) List(ctx context.Context, options *internalversion.L return nil, err } ns := requestK8s.NamespaceValue(ctx) - if user.GetIdentityType() == authlib.TypeAccessPolicy { - user = nil // nill user can see everything + if user.GetIdentityType() != authlib.TypeUser { + return nil, fmt.Errorf("only users may list preferences") } return s.sql.ListPreferences(ctx, ns, user, true) } @@ -108,8 +108,6 @@ func (s *preferenceStorage) Get(ctx context.Context, name string, options *metav default: return false, fmt.Errorf("unsupported name") } - }, func(p *preferenceModel) bool { - return true }) if err != nil { return nil, err diff --git a/pkg/registry/apis/preferences/legacy/queries.go b/pkg/registry/apis/preferences/legacy/queries.go index 8ec075ec6def2..0e35e2b672d78 100644 --- a/pkg/registry/apis/preferences/legacy/queries.go +++ b/pkg/registry/apis/preferences/legacy/queries.go @@ -38,6 +38,7 @@ type preferencesQuery struct { UserUID string UserTeams []string // also requires user UID TeamUID string + All bool // explicitly request all preferences UserTable string TeamTable string @@ -59,6 +60,12 @@ func (r preferencesQuery) Validate() error { if len(r.UserTeams) > 0 && r.UserUID == "" { return fmt.Errorf("user required when filtering by a set of teams") } + if r.UserUID == "" && r.TeamUID == "" { + if r.All { + return nil // OK + } + return fmt.Errorf("to list all preferences, explicitly set the .All flag") + } return nil } diff --git a/pkg/registry/apis/preferences/legacy/queries_test.go b/pkg/registry/apis/preferences/legacy/queries_test.go index 8b4856a04dcce..58587fad02dde 100644 --- a/pkg/registry/apis/preferences/legacy/queries_test.go +++ b/pkg/registry/apis/preferences/legacy/queries_test.go @@ -9,7 +9,7 @@ import ( "github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate/mocks" ) -func TestStarsQueries(t *testing.T) { +func TestPreferencesQueries(t *testing.T) { // prefix tables with grafana nodb := &legacysql.LegacyDatabaseHelper{ Table: func(n string) string { @@ -35,9 +35,28 @@ func TestStarsQueries(t *testing.T) { SQLTemplatesFS: sqlTemplatesFS, Templates: map[*template.Template][]mocks.TemplateTestCase{ sqlPreferencesQuery: { + { + Name: "all-error", + Data: getPreferencesQuery(1, func(q *preferencesQuery) {}), + ValidationError: "to list all preferences, explicitly set the .All flag", + }, + { + Name: "missing-org", + Data: getPreferencesQuery(0, func(q *preferencesQuery) {}), + ValidationError: "must include an orgID", + }, + { + Name: "missing-user", + Data: getPreferencesQuery(1, func(q *preferencesQuery) { + q.UserTeams = []string{"a"} + }), + ValidationError: "user required when filtering by a set of teams", + }, { Name: "all", - Data: getPreferencesQuery(1, func(q *preferencesQuery) {}), + Data: getPreferencesQuery(1, func(q *preferencesQuery) { + q.All = true + }), }, { Name: "user-no-teams", @@ -69,7 +88,9 @@ func TestStarsQueries(t *testing.T) { sqlPreferencesRV: { { Name: "get", - Data: getPreferencesQuery(1, func(q *preferencesQuery) {}), + Data: getPreferencesQuery(1, func(q *preferencesQuery) { + q.All = true // avoid validation error + }), }, }, sqlTeams: { diff --git a/pkg/registry/apis/preferences/legacy/sql.go b/pkg/registry/apis/preferences/legacy/sql.go index 845dc3f0bd03c..f87e3edbe80f7 100644 --- a/pkg/registry/apis/preferences/legacy/sql.go +++ b/pkg/registry/apis/preferences/legacy/sql.go @@ -9,6 +9,7 @@ import ( "time" authlib "github.com/grafana/authlib/types" + "github.com/grafana/grafana-app-sdk/logging" preferences "github.com/grafana/grafana/apps/preferences/pkg/apis/preferences/v1alpha1" "github.com/grafana/grafana/pkg/apimachinery/identity" pref "github.com/grafana/grafana/pkg/services/preference" @@ -43,7 +44,6 @@ func NewLegacySQL(db legacysql.LegacyDatabaseProvider) *LegacySQL { func (s *LegacySQL) listPreferences(ctx context.Context, ns string, orgId int64, cb func(req *preferencesQuery) (bool, error), - access func(p *preferenceModel) bool, ) ([]preferences.Preferences, int64, error) { var results []preferences.Preferences var rv sql.NullTime @@ -65,6 +65,9 @@ func (s *LegacySQL) listPreferences(ctx context.Context, return nil, 0, fmt.Errorf("execute template %q: %w", sqlPreferencesQuery.Name(), err) } + // Debug the SQL query + logging.FromContext(ctx).Info("ListPreferences", "query", q, "args", req.GetArgs()) + sess := sql.DB.GetSqlxSession() rows, err := sess.Query(ctx, q, req.GetArgs()...) defer func() { @@ -99,14 +102,12 @@ func (s *LegacySQL) listPreferences(ctx context.Context, if pref.Updated.After(rv.Time) { rv.Time = pref.Updated } - if !access(&pref) { - continue // user does not have access - } results = append(results, asPreferencesResource(ns, &pref)) } if needsRV { req.Reset() + req.All = true // Avoid validation error q, err = sqltemplate.Execute(sqlPreferencesRV, req) if err != nil { return nil, 0, fmt.Errorf("execute template %q: %w", sqlPreferencesRV.Name(), err) @@ -127,6 +128,7 @@ func (s *LegacySQL) listPreferences(ctx context.Context, return results, rv.Time.UnixMilli(), err } +// Note sending a null user will list all preferences in the namespace! func (s *LegacySQL) ListPreferences(ctx context.Context, ns string, user identity.Requester, needsRV bool) (*preferences.PreferencesList, error) { if ns == "" { return nil, fmt.Errorf("namespace is required") @@ -137,7 +139,6 @@ func (s *LegacySQL) ListPreferences(ctx context.Context, ns string, user identit return nil, err } - // when the user is nil, it is actually admin and can see everything var teams []string found, rv, err := s.listPreferences(ctx, ns, info.OrgID, func(req *preferencesQuery) (bool, error) { @@ -148,21 +149,11 @@ func (s *LegacySQL) ListPreferences(ctx context.Context, ns string, user identit UserUID: req.UserUID, }, false) req.UserTeams = teams + } else { + req.All = true } return needsRV, err }, - func(p *preferenceModel) bool { - if user == nil || user.GetIsGrafanaAdmin() { - return true - } - if p.UserUID.String != "" { - return user.GetIdentifier() == p.UserUID.String - } - if p.TeamUID.String != "" { - return slices.Contains(teams, p.TeamUID.String) - } - return true - }, ) if err != nil { return nil, err diff --git a/pkg/registry/apis/preferences/legacy/sql_preferences_query.sql b/pkg/registry/apis/preferences/legacy/sql_preferences_query.sql index 4f88bea3482b6..57e18826d3423 100644 --- a/pkg/registry/apis/preferences/legacy/sql_preferences_query.sql +++ b/pkg/registry/apis/preferences/legacy/sql_preferences_query.sql @@ -19,8 +19,10 @@ WHERE p.org_id = {{ .Arg .OrgID }} {{ if .HasTeams }} OR t.uid IN ({{ .ArgList .UserTeams }}) {{ end }} - OR p.user_id = 0 + OR (p.user_id = 0 AND p.team_id = 0) {{ end }} ) +{{ else if not .All }} + invalid query -- specify All to list all permissions in query {{ end }} ORDER BY p.user_id asc, p.team_id asc, p.org_id asc diff --git a/pkg/registry/apis/preferences/legacy/testdata/mysql--sql_preferences_query-current.sql b/pkg/registry/apis/preferences/legacy/testdata/mysql--sql_preferences_query-current.sql index fc9c4244d9570..6e244efd70a94 100755 --- a/pkg/registry/apis/preferences/legacy/testdata/mysql--sql_preferences_query-current.sql +++ b/pkg/registry/apis/preferences/legacy/testdata/mysql--sql_preferences_query-current.sql @@ -13,6 +13,6 @@ SELECT p.id, p.org_id, WHERE p.org_id = 1 AND (u.uid = 'uuu' OR t.uid IN ('a', 'b', 'c') - OR p.user_id = 0 + OR (p.user_id = 0 AND p.team_id = 0) ) ORDER BY p.user_id asc, p.team_id asc, p.org_id asc diff --git a/pkg/registry/apis/preferences/legacy/testdata/mysql--sql_preferences_query-user-no-teams.sql b/pkg/registry/apis/preferences/legacy/testdata/mysql--sql_preferences_query-user-no-teams.sql index b1a5757d95383..73a25b0929973 100755 --- a/pkg/registry/apis/preferences/legacy/testdata/mysql--sql_preferences_query-user-no-teams.sql +++ b/pkg/registry/apis/preferences/legacy/testdata/mysql--sql_preferences_query-user-no-teams.sql @@ -12,6 +12,6 @@ SELECT p.id, p.org_id, LEFT JOIN `grafana`.`team` as t ON p.team_id = t.id WHERE p.org_id = 1 AND (u.uid = 'uuu' - OR p.user_id = 0 + OR (p.user_id = 0 AND p.team_id = 0) ) ORDER BY p.user_id asc, p.team_id asc, p.org_id asc diff --git a/pkg/registry/apis/preferences/legacy/testdata/postgres--sql_preferences_query-current.sql b/pkg/registry/apis/preferences/legacy/testdata/postgres--sql_preferences_query-current.sql index fb378535fe04d..b6b07a05162aa 100755 --- a/pkg/registry/apis/preferences/legacy/testdata/postgres--sql_preferences_query-current.sql +++ b/pkg/registry/apis/preferences/legacy/testdata/postgres--sql_preferences_query-current.sql @@ -13,6 +13,6 @@ SELECT p.id, p.org_id, WHERE p.org_id = 1 AND (u.uid = 'uuu' OR t.uid IN ('a', 'b', 'c') - OR p.user_id = 0 + OR (p.user_id = 0 AND p.team_id = 0) ) ORDER BY p.user_id asc, p.team_id asc, p.org_id asc diff --git a/pkg/registry/apis/preferences/legacy/testdata/postgres--sql_preferences_query-user-no-teams.sql b/pkg/registry/apis/preferences/legacy/testdata/postgres--sql_preferences_query-user-no-teams.sql index f81506ea85329..d9159a963c153 100755 --- a/pkg/registry/apis/preferences/legacy/testdata/postgres--sql_preferences_query-user-no-teams.sql +++ b/pkg/registry/apis/preferences/legacy/testdata/postgres--sql_preferences_query-user-no-teams.sql @@ -12,6 +12,6 @@ SELECT p.id, p.org_id, LEFT JOIN "grafana"."team" as t ON p.team_id = t.id WHERE p.org_id = 1 AND (u.uid = 'uuu' - OR p.user_id = 0 + OR (p.user_id = 0 AND p.team_id = 0) ) ORDER BY p.user_id asc, p.team_id asc, p.org_id asc diff --git a/pkg/registry/apis/preferences/legacy/testdata/sqlite--sql_preferences_query-current.sql b/pkg/registry/apis/preferences/legacy/testdata/sqlite--sql_preferences_query-current.sql index fb378535fe04d..b6b07a05162aa 100755 --- a/pkg/registry/apis/preferences/legacy/testdata/sqlite--sql_preferences_query-current.sql +++ b/pkg/registry/apis/preferences/legacy/testdata/sqlite--sql_preferences_query-current.sql @@ -13,6 +13,6 @@ SELECT p.id, p.org_id, WHERE p.org_id = 1 AND (u.uid = 'uuu' OR t.uid IN ('a', 'b', 'c') - OR p.user_id = 0 + OR (p.user_id = 0 AND p.team_id = 0) ) ORDER BY p.user_id asc, p.team_id asc, p.org_id asc diff --git a/pkg/registry/apis/preferences/legacy/testdata/sqlite--sql_preferences_query-user-no-teams.sql b/pkg/registry/apis/preferences/legacy/testdata/sqlite--sql_preferences_query-user-no-teams.sql index f81506ea85329..d9159a963c153 100755 --- a/pkg/registry/apis/preferences/legacy/testdata/sqlite--sql_preferences_query-user-no-teams.sql +++ b/pkg/registry/apis/preferences/legacy/testdata/sqlite--sql_preferences_query-user-no-teams.sql @@ -12,6 +12,6 @@ SELECT p.id, p.org_id, LEFT JOIN "grafana"."team" as t ON p.team_id = t.id WHERE p.org_id = 1 AND (u.uid = 'uuu' - OR p.user_id = 0 + OR (p.user_id = 0 AND p.team_id = 0) ) ORDER BY p.user_id asc, p.team_id asc, p.org_id asc diff --git a/pkg/storage/unified/sql/sqltemplate/mocks/test_snapshots.go b/pkg/storage/unified/sql/sqltemplate/mocks/test_snapshots.go index fa1b8c1232759..d024ed880f600 100644 --- a/pkg/storage/unified/sql/sqltemplate/mocks/test_snapshots.go +++ b/pkg/storage/unified/sql/sqltemplate/mocks/test_snapshots.go @@ -73,6 +73,9 @@ type TemplateTestCase struct { // Data should be the struct passed to the template. Data sqltemplate.SQLTemplate + + // If the query should not validate + ValidationError string } type TemplateTestSetup struct { @@ -122,8 +125,12 @@ func CheckQuerySnapshots(t *testing.T, setup TemplateTestSetup) { // but also not worth deep cloning input.Data.SetDialect(dialect) err := input.Data.Validate() - + if input.ValidationError != "" { + require.ErrorContains(t, err, input.ValidationError) + return + } require.NoError(t, err) + got, err := sqltemplate.Execute(tmpl, input.Data) require.NoError(t, err)