From 586cc1d38ce3b88034e688b894cda0a6ddc1d278 Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Mon, 15 Mar 2021 11:11:56 -0400 Subject: [PATCH] Merge pull request from GHSA-5v95-v8c8-3rh6 This fixes a security vulnerability where, with a carefully crafted request or malicious proxy, a user with UserWrite permissions could create another user with higher privileges than their own due to insufficient checks on the allowed set of permissions. --- pkg/rbac/rbac.go | 10 ++++++++++ pkg/rbac/rbac_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/pkg/rbac/rbac.go b/pkg/rbac/rbac.go index 18c433bd1..94a0c9276 100644 --- a/pkg/rbac/rbac.go +++ b/pkg/rbac/rbac.go @@ -64,6 +64,16 @@ func Can(given Permission, target Permission) bool { func CompileAndAuthorize(actorPermission Permission, toUpdate []Permission) (Permission, error) { var permission Permission for _, update := range toUpdate { + // Verify the provided permission is a known permission. This prevents a + // security vulnerability whereby a carefully crafted request is able to + // provide a value that correctly passes an the bitwise AND check and then + // modifies the target permission using OR to escalate privilege. + if _, ok := PermissionMap[update]; !ok { + if update != LegacyRealmAdmin && update != LegacyRealmUser { + return 0, fmt.Errorf("provided permission %v is unknown", update) + } + } + // Verify that the user making changes has the permissions they are trying // to grant. It is not valid for someone to grant permissions larger than // they currently have. diff --git a/pkg/rbac/rbac_test.go b/pkg/rbac/rbac_test.go index d5e3dd2e6..d68bc01c0 100644 --- a/pkg/rbac/rbac_test.go +++ b/pkg/rbac/rbac_test.go @@ -57,6 +57,30 @@ func TestRequiredPermissions(t *testing.T) { t.Errorf("expected error") } }) + + t.Run("legacy_admin", func(t *testing.T) { + t.Parallel() + + if _, err := CompileAndAuthorize(LegacyRealmAdmin, []Permission{LegacyRealmAdmin}); err != nil { + t.Error(err) + } + }) + + t.Run("legacy_user", func(t *testing.T) { + t.Parallel() + + if _, err := CompileAndAuthorize(LegacyRealmAdmin, []Permission{LegacyRealmUser}); err != nil { + t.Error(err) + } + }) + + t.Run("escalate", func(t *testing.T) { + t.Parallel() + + if _, err := CompileAndAuthorize(UserRead|UserWrite, []Permission{16383}); err == nil { + t.Errorf("expected error") + } + }) } func TestImpliedBy(t *testing.T) {