fix: sanitize permissions

.changeset/entries/6e78e6f7f9486799935df288f3d1467a646116fddc2952e79e559b1d4cc58d79.yaml

@@ -0,0 +1,6 @@
+type: fix
+module: x/subspaces
+pull_request: 801
+description: Added permissions sanitization
+backward_compatible: true
+date: 2022-04-01T09:09:14.889652713Z

testutil/context.go

@@ -0,0 +1,32 @@
+package testutil
+
+import (
+ "github.com/cosmos/cosmos-sdk/store"
+ sdk "github.com/cosmos/cosmos-sdk/types"
+ "github.com/tendermint/tendermint/libs/log"
+ tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
+ dbm "github.com/tendermint/tm-db"
+)
+
+func BuildContext(
+ keys map[string]*sdk.KVStoreKey, tKeys map[string]*sdk.TransientStoreKey, memKeys map[string]*sdk.MemoryStoreKey,
+) sdk.Context {
+ db := dbm.NewMemDB()
+ cms := store.NewCommitMultiStore(db)
+ for _, key := range keys {
+ cms.MountStoreWithDB(key, sdk.StoreTypeIAVL, db)
+ }
+ for _, tKey := range tKeys {
+ cms.MountStoreWithDB(tKey, sdk.StoreTypeTransient, db)
+ }
+ for _, memKey := range memKeys {
+ cms.MountStoreWithDB(memKey, sdk.StoreTypeMemory, nil)
+ }
+
+ err := cms.LoadLatestVersion()
+ if err != nil {
+ panic(err)
+ }
+
+ return sdk.NewContext(cms, tmproto.Header{}, false, log.NewNopLogger())
+}

x/profiles/legacy/v5/store_test.go

@@ -12,44 +12,16 @@ import (
  "github.com/desmos-labs/desmos/v3/testutil"
  profilestypes "github.com/desmos-labs/desmos/v3/x/profiles/types"
 
- "github.com/cosmos/cosmos-sdk/store"
+ sdk "github.com/cosmos/cosmos-sdk/types"
  authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
  capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
  paramstypes "github.com/cosmos/cosmos-sdk/x/params/types"
- "github.com/tendermint/tendermint/libs/log"
- tmproto "github.com/tendermint/tendermint/proto/tendermint/types"
- dbm "github.com/tendermint/tm-db"
-
- sdk "github.com/cosmos/cosmos-sdk/types"
  "github.com/stretchr/testify/require"
 
  "github.com/desmos-labs/desmos/v3/app"
  "github.com/desmos-labs/desmos/v3/x/relationships/types"
 )
 
-func buildContext(
- keys map[string]*sdk.KVStoreKey, tKeys map[string]*sdk.TransientStoreKey, memKeys map[string]*sdk.MemoryStoreKey,
-) sdk.Context {
- db := dbm.NewMemDB()
- cms := store.NewCommitMultiStore(db)
- for _, key := range keys {
- cms.MountStoreWithDB(key, sdk.StoreTypeIAVL, db)
- }
- for _, tKey := range tKeys {
- cms.MountStoreWithDB(tKey, sdk.StoreTypeTransient, db)
- }
- for _, memKey := range memKeys {
- cms.MountStoreWithDB(memKey, sdk.StoreTypeMemory, nil)
- }
-
- err := cms.LoadLatestVersion()
- if err != nil {
- panic(err)
- }
-
- return sdk.NewContext(cms, tmproto.Header{}, false, log.NewNopLogger())
-}
-
 func TestMigrateStore(t *testing.T) {
  cdc, _ := app.MakeCodecs()
 
@@ -154,7 +126,7 @@ func TestMigrateStore(t *testing.T) {
  for _, tc := range testCases {
  tc := tc
  t.Run(tc.name, func(t *testing.T) {
- ctx := buildContext(keys, tKeys, memKeys)
+ ctx := testutil.BuildContext(keys, tKeys, memKeys)
  if tc.store != nil {
  tc.store(ctx)
  }

x/subspaces/keeper/migrations.go

@@ -0,0 +1,26 @@
+package keeper
+
+import (
+ sdk "github.com/cosmos/cosmos-sdk/types"
+
+ v2 "github.com/desmos-labs/desmos/v3/x/subspaces/legacy/v2"
+)
+
+// DONTCOVER
+
+// Migrator is a struct for handling in-place store migrations.
+type Migrator struct {
+ keeper Keeper
+}
+
+// NewMigrator returns a new Migrator
+func NewMigrator(keeper Keeper) Migrator {
+ return Migrator{
+ keeper: keeper,
+ }
+}
+
+// Migrate1to2 migrates from version 1 to 2.
+func (m Migrator) Migrate1to2(ctx sdk.Context) error {
+ return v2.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc)
+}

x/subspaces/keeper/msg_server.go

@@ -276,6 +276,11 @@ func (k msgServer) SetUserGroupPermissions(goCtx context.Context, msg *types.Msg
  return nil, sdkerrors.Wrapf(types.ErrPermissionDenied, "you cannot manage permissions in this subspace")
  }
 
+ // Make sure the permission is valid
+ if !types.IsPermissionValid(msg.Permissions) {
+ return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "invalid permission value")
+ }
+
  // Set the group permissions and store the group
  group.Permissions = msg.Permissions
  k.SaveUserGroup(ctx, group)
@@ -474,6 +479,11 @@ func (k msgServer) SetUserPermissions(goCtx context.Context, msg *types.MsgSetUs
  return nil, sdkerrors.Wrapf(types.ErrPermissionDenied, "you cannot manage permissions in this subspace")
  }
 
+ // Make sure the permission is valid
+ if !types.IsPermissionValid(msg.Permissions) {
+ return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "invalid permission value")
+ }
+
  // Set the permissions
  if msg.Permissions == types.PermissionNothing {
  // Remove the permission to clear the store if PermissionNothing is used

x/subspaces/keeper/msg_server_test.go

@@ -914,7 +914,39 @@ func (suite *KeeperTestsuite) TestMsgServer_SetUserGroupPermissions() {
  shouldErr: true,
  },
  {
- name: "existing group is deleted properly",
+ name: "invalid permission value returns error",
+ store: func(ctx sdk.Context) {
+ suite.k.SaveSubspace(ctx, types.NewSubspace(
+ 1,
+ "Test subspace",
+ "This is a test subspace",
+ "cosmos1qzskhrcjnkdz2ln4yeafzsdwht8ch08j4wed69",
+ "cosmos1m0czrla04f7rp3zg7dsgc4kla54q7pc4xt00l5",
+ "cosmos1qzskhrcjnkdz2ln4yeafzsdwht8ch08j4wed69",
+ time.Date(2020, 1, 1, 12, 00, 00, 000, time.UTC),
+ ))
+ suite.k.SaveUserGroup(ctx, types.NewUserGroup(
+ 1,
+ 1,
+ "Test group",
+ "This is a test group",
+ types.PermissionWrite,
+ ))
+
+ sdkAddr, err := sdk.AccAddressFromBech32("cosmos1x5pjlvufs4znnhhkwe8v4tw3kz30f3lxgwza53")
+ suite.Require().NoError(err)
+ suite.k.SetUserPermissions(ctx, 1, sdkAddr, types.PermissionSetPermissions)
+ },
+ msg: types.NewMsgSetUserGroupPermissions(
+ 1,
+ 1,
+ 256,
+ "cosmos1x5pjlvufs4znnhhkwe8v4tw3kz30f3lxgwza53",
+ ),
+ shouldErr: true,
+ },
+ {
+ name: "group permissions are updated correctly",
  store: func(ctx sdk.Context) {
  suite.k.SaveSubspace(ctx, types.NewSubspace(
  1,
@@ -1535,6 +1567,31 @@ func (suite *KeeperTestsuite) TestMsgServer_SetUserPermissions() {
  ),
  shouldErr: true,
  },
+ {
+ name: "invalid permission returns error",
+ store: func(ctx sdk.Context) {
+ suite.k.SaveSubspace(ctx, types.NewSubspace(
+ 1,
+ "Test subspace",
+ "This is a test subspace",
+ "cosmos1qzskhrcjnkdz2ln4yeafzsdwht8ch08j4wed69",
+ "cosmos1m0czrla04f7rp3zg7dsgc4kla54q7pc4xt00l5",
+ "cosmos1qzskhrcjnkdz2ln4yeafzsdwht8ch08j4wed69",
+ time.Date(2020, 1, 1, 12, 00, 00, 000, time.UTC),
+ ))
+
+ sdkAddr, err := sdk.AccAddressFromBech32("cosmos1x5pjlvufs4znnhhkwe8v4tw3kz30f3lxgwza53")
+ suite.Require().NoError(err)
+ suite.k.SetUserPermissions(ctx, 1, sdkAddr, types.PermissionSetPermissions)
+ },
+ msg: types.NewMsgSetUserPermissions(
+ 1,
+ "cosmos1x5pjlvufs4znnhhkwe8v4tw3kz30f3lxgwza53",
+ 256,
+ "cosmos1x5pjlvufs4znnhhkwe8v4tw3kz30f3lxgwza53",
+ ),
+ shouldErr: true,
+ },
  {
  name: "permissions are set correctly",
  store: func(ctx sdk.Context) {

x/subspaces/legacy/v2/store.go

@@ -0,0 +1,87 @@
+package v2
+
+import (
+ "github.com/cosmos/cosmos-sdk/codec"
+ "github.com/cosmos/cosmos-sdk/store/prefix"
+ sdk "github.com/cosmos/cosmos-sdk/types"
+
+ "github.com/desmos-labs/desmos/v3/x/subspaces/types"
+)
+
+func MigrateStore(ctx sdk.Context, storeKey sdk.StoreKey, cdc codec.BinaryCodec) error {
+ store := ctx.KVStore(storeKey)
+
+ err := fixGroupsPermissions(store, cdc)
+ if err != nil {
+ return err
+ }
+
+ fixUsersPermissions(store)
+
+ return nil
+}
+
+func fixGroupsPermissions(store sdk.KVStore, cdc codec.BinaryCodec) error {
+ groupsStore := prefix.NewStore(store, types.GroupsPrefix)
+ iterator := groupsStore.Iterator(nil, nil)
+
+ var groups []types.UserGroup
+ for ; iterator.Valid(); iterator.Next() {
+ var group types.UserGroup
+ err := cdc.Unmarshal(iterator.Value(), &group)
+ if err != nil {
+ return err
+ }
+
+ // Sanitize the permissions
+ group.Permissions = types.SanitizePermission(group.Permissions)
+ groups = append(groups, group)
+ }
+
+ iterator.Close()
+
+ // Store the new groups
+ for i, group := range groups {
+ bz, err := cdc.Marshal(&groups[i])
+ if err != nil {
+ return err
+ }
+
+ store.Set(types.GroupStoreKey(group.SubspaceID, group.ID), bz)
+ }
+
+ return nil
+}
+
+type userPermissionDetails struct {
+ subspaceID uint64
+ user sdk.AccAddress
+ permissions types.Permission
+}
+
+func fixUsersPermissions(store sdk.KVStore) {
+ permissionsStore := prefix.NewStore(store, types.UserPermissionsStorePrefix)
+ iterator := permissionsStore.Iterator(nil, nil)
+
+ var permissions []userPermissionDetails
+ for ; iterator.Valid(); iterator.Next() {
+ // The first 8 bytes are the subspace id (uint64 takes up 8 bytes)
+ // The remaining bytes are the user address
+ subspaceBz, addressBz := iterator.Key()[:8], iterator.Key()[8:]
+
+ permissions = append(permissions, userPermissionDetails{
+ subspaceID: types.GetSubspaceIDFromBytes(subspaceBz),
+ user: types.GetAddressBytes(addressBz),
+
+ // Sanitize the permission
+ permissions: types.SanitizePermission(types.UnmarshalPermission(iterator.Value())),
+ })
+ }
+
+ iterator.Close()
+
+ // Store the new permissions
+ for _, entry := range permissions {
+ store.Set(types.UserPermissionStoreKey(entry.subspaceID, entry.user), types.MarshalPermission(entry.permissions))
+ }
+}

x/subspaces/legacy/v2/store_test.go

@@ -0,0 +1,90 @@
+package v2_test
+
+import (
+ "testing"
+
+ sdk "github.com/cosmos/cosmos-sdk/types"
+ capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
+ paramstypes "github.com/cosmos/cosmos-sdk/x/params/types"
+ "github.com/stretchr/testify/require"
+
+ "github.com/desmos-labs/desmos/v3/app"
+ "github.com/desmos-labs/desmos/v3/testutil"
+ v2 "github.com/desmos-labs/desmos/v3/x/subspaces/legacy/v2"
+ "github.com/desmos-labs/desmos/v3/x/subspaces/types"
+)
+
+func TestMigrateStore(t *testing.T) {
+ cdc, _ := app.MakeCodecs()
+
+ // Build all the necessary keys
+ keys := sdk.NewKVStoreKeys(types.StoreKey)
+ tKeys := sdk.NewTransientStoreKeys(paramstypes.TStoreKey)
+ memKeys := sdk.NewMemoryStoreKeys(capabilitytypes.MemStoreKey)
+
+ testCases := []struct {
+ name string
+ store func(ctx sdk.Context)
+ shouldErr bool
+ check func(ctx sdk.Context)
+ }{
+ {
+ name: "groups permissions are sanitized properly",
+ store: func(ctx sdk.Context) {
+ kvStore := ctx.KVStore(keys[types.StoreKey])
+
+ group := types.NewUserGroup(11, 11, "Test group", "", 0b11111111111111111111111111000001)
+ kvStore.Set(types.GroupStoreKey(group.SubspaceID, group.ID), cdc.MustMarshal(&group))
+ },
+ check: func(ctx sdk.Context) {
+ kvStore := ctx.KVStore(keys[types.StoreKey])
+
+ // Check the permissions
+ var group types.UserGroup
+ cdc.MustUnmarshal(kvStore.Get(types.GroupStoreKey(11, 11)), &group)
+ require.Equal(t, types.PermissionWrite, group.Permissions)
+ },
+ },
+ {
+ name: "user permissions are sanitized properly",
+ store: func(ctx sdk.Context) {
+ kvStore := ctx.KVStore(keys[types.StoreKey])
+
+ addr, err := sdk.AccAddressFromBech32("cosmos12e7ejq92sma437d3svemgfvl8sul8lxfs69mjv")
+ require.NoError(t, err)
+
+ kvStore.Set(types.UserPermissionStoreKey(11, addr), types.MarshalPermission(0b11111111111111111111111111000001))
+ },
+ check: func(ctx sdk.Context) {
+ kvStore := ctx.KVStore(keys[types.StoreKey])
+
+ addr, err := sdk.AccAddressFromBech32("cosmos12e7ejq92sma437d3svemgfvl8sul8lxfs69mjv")
+ require.NoError(t, err)
+
+ // Check the permissions
+ stored := types.UnmarshalPermission(kvStore.Get(types.UserPermissionStoreKey(11, addr)))
+ require.Equal(t, types.PermissionWrite, stored)
+ },
+ },
+ }
+
+ for _, tc := range testCases {
+ tc := tc
+ t.Run(tc.name, func(t *testing.T) {
+ ctx := testutil.BuildContext(keys, tKeys, memKeys)
+ if tc.store != nil {
+ tc.store(ctx)
+ }
+
+ err := v2.MigrateStore(ctx, keys[types.StoreKey], cdc)
+ if tc.shouldErr {
+ require.Error(t, err)
+ } else {
+ require.NoError(t, err)
+ if tc.check != nil {
+ tc.check(ctx)
+ }
+ }
+ })
+ }
+}