Capability Fixes (#7918)
* add cap and name checks * Update x/capability/keeper/keeper.go * address reviews * fix missing trim Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
This commit is contained in:
parent
15103b63a1
commit
4b529a41cf
|
@ -65,6 +65,9 @@ Ref: https://keepachangelog.com/en/1.0.0/
|
||||||
* `server/types.AppExporter` requires extra argument: `AppOptions`.
|
* `server/types.AppExporter` requires extra argument: `AppOptions`.
|
||||||
* `server.AddCommands` requires extra argument: `addStartFlags types.ModuleInitFlags`
|
* `server.AddCommands` requires extra argument: `addStartFlags types.ModuleInitFlags`
|
||||||
* `x/crisis.NewAppModule` has a new attribute: `skipGenesisInvariants`. [PR](https://github.com/cosmos/cosmos-sdk/pull/7764)
|
* `x/crisis.NewAppModule` has a new attribute: `skipGenesisInvariants`. [PR](https://github.com/cosmos/cosmos-sdk/pull/7764)
|
||||||
|
* [#7918](https://github.com/cosmos/cosmos-sdk/pull/7918) Add x/capability safety checks:
|
||||||
|
* All outward facing APIs will now check that capability is not nil and name is not empty before performing any state-machine changes
|
||||||
|
* `SetIndex` has been renamed to `InitializeIndex`
|
||||||
|
|
||||||
### Features
|
### Features
|
||||||
|
|
||||||
|
|
|
@ -9,7 +9,7 @@ import (
|
||||||
// InitGenesis initializes the capability module's state from a provided genesis
|
// InitGenesis initializes the capability module's state from a provided genesis
|
||||||
// state.
|
// state.
|
||||||
func InitGenesis(ctx sdk.Context, k keeper.Keeper, genState types.GenesisState) {
|
func InitGenesis(ctx sdk.Context, k keeper.Keeper, genState types.GenesisState) {
|
||||||
if err := k.SetIndex(ctx, genState.Index); err != nil {
|
if err := k.InitializeIndex(ctx, genState.Index); err != nil {
|
||||||
panic(err)
|
panic(err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -2,6 +2,7 @@ package keeper
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"strings"
|
||||||
|
|
||||||
"github.com/tendermint/tendermint/libs/log"
|
"github.com/tendermint/tendermint/libs/log"
|
||||||
|
|
||||||
|
@ -49,6 +50,8 @@ type (
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// NewKeeper constructs a new CapabilityKeeper instance and initializes maps
|
||||||
|
// for capability map and scopedModules map.
|
||||||
func NewKeeper(cdc codec.BinaryMarshaler, storeKey, memKey sdk.StoreKey) *Keeper {
|
func NewKeeper(cdc codec.BinaryMarshaler, storeKey, memKey sdk.StoreKey) *Keeper {
|
||||||
return &Keeper{
|
return &Keeper{
|
||||||
cdc: cdc,
|
cdc: cdc,
|
||||||
|
@ -67,6 +70,9 @@ func (k *Keeper) ScopeToModule(moduleName string) ScopedKeeper {
|
||||||
if k.sealed {
|
if k.sealed {
|
||||||
panic("cannot scope to module via a sealed capability keeper")
|
panic("cannot scope to module via a sealed capability keeper")
|
||||||
}
|
}
|
||||||
|
if strings.TrimSpace(moduleName) == "" {
|
||||||
|
panic("cannot scope to an empty module name")
|
||||||
|
}
|
||||||
|
|
||||||
if _, ok := k.scopedModules[moduleName]; ok {
|
if _, ok := k.scopedModules[moduleName]; ok {
|
||||||
panic(fmt.Sprintf("cannot create multiple scoped keepers for the same module name: %s", moduleName))
|
panic(fmt.Sprintf("cannot create multiple scoped keepers for the same module name: %s", moduleName))
|
||||||
|
@ -117,10 +123,10 @@ func (k *Keeper) InitializeAndSeal(ctx sdk.Context) {
|
||||||
k.sealed = true
|
k.sealed = true
|
||||||
}
|
}
|
||||||
|
|
||||||
// SetIndex sets the index to one (or greater) in InitChain according
|
// InitializeIndex sets the index to one (or greater) in InitChain according
|
||||||
// to the GenesisState. It must only be called once.
|
// to the GenesisState. It must only be called once.
|
||||||
// It will panic if the provided index is 0, or if the index is already set.
|
// It will panic if the provided index is 0, or if the index is already set.
|
||||||
func (k Keeper) SetIndex(ctx sdk.Context, index uint64) error {
|
func (k Keeper) InitializeIndex(ctx sdk.Context, index uint64) error {
|
||||||
if index == 0 {
|
if index == 0 {
|
||||||
panic("SetIndex requires index > 0")
|
panic("SetIndex requires index > 0")
|
||||||
}
|
}
|
||||||
|
@ -200,6 +206,9 @@ func (k Keeper) InitializeCapability(ctx sdk.Context, index uint64, owners types
|
||||||
// Note, namespacing is completely local, which is safe since records are prefixed
|
// Note, namespacing is completely local, which is safe since records are prefixed
|
||||||
// with the module name and no two ScopedKeeper can have the same module name.
|
// with the module name and no two ScopedKeeper can have the same module name.
|
||||||
func (sk ScopedKeeper) NewCapability(ctx sdk.Context, name string) (*types.Capability, error) {
|
func (sk ScopedKeeper) NewCapability(ctx sdk.Context, name string) (*types.Capability, error) {
|
||||||
|
if strings.TrimSpace(name) == "" {
|
||||||
|
return nil, sdkerrors.Wrap(types.ErrInvalidCapabilityName, "capability name cannot be empty")
|
||||||
|
}
|
||||||
store := ctx.KVStore(sk.storeKey)
|
store := ctx.KVStore(sk.storeKey)
|
||||||
|
|
||||||
if _, ok := sk.GetCapability(ctx, name); ok {
|
if _, ok := sk.GetCapability(ctx, name); ok {
|
||||||
|
@ -247,6 +256,9 @@ func (sk ScopedKeeper) NewCapability(ctx sdk.Context, name string) (*types.Capab
|
||||||
// Note, the capability's forward mapping is indexed by a string which should
|
// Note, the capability's forward mapping is indexed by a string which should
|
||||||
// contain its unique memory reference.
|
// contain its unique memory reference.
|
||||||
func (sk ScopedKeeper) AuthenticateCapability(ctx sdk.Context, cap *types.Capability, name string) bool {
|
func (sk ScopedKeeper) AuthenticateCapability(ctx sdk.Context, cap *types.Capability, name string) bool {
|
||||||
|
if strings.TrimSpace(name) == "" || cap == nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
return sk.GetCapabilityName(ctx, cap) == name
|
return sk.GetCapabilityName(ctx, cap) == name
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -256,6 +268,12 @@ func (sk ScopedKeeper) AuthenticateCapability(ctx sdk.Context, cap *types.Capabi
|
||||||
// index. If the owner already exists, it will return an error. Otherwise, it will
|
// index. If the owner already exists, it will return an error. Otherwise, it will
|
||||||
// also set a forward and reverse index for the capability and capability name.
|
// also set a forward and reverse index for the capability and capability name.
|
||||||
func (sk ScopedKeeper) ClaimCapability(ctx sdk.Context, cap *types.Capability, name string) error {
|
func (sk ScopedKeeper) ClaimCapability(ctx sdk.Context, cap *types.Capability, name string) error {
|
||||||
|
if cap == nil {
|
||||||
|
return sdkerrors.Wrap(types.ErrNilCapability, "cannot claim nil capability")
|
||||||
|
}
|
||||||
|
if strings.TrimSpace(name) == "" {
|
||||||
|
return sdkerrors.Wrap(types.ErrInvalidCapabilityName, "capability name cannot be empty")
|
||||||
|
}
|
||||||
// update capability owner set
|
// update capability owner set
|
||||||
if err := sk.addOwner(ctx, cap, name); err != nil {
|
if err := sk.addOwner(ctx, cap, name); err != nil {
|
||||||
return err
|
return err
|
||||||
|
@ -282,6 +300,9 @@ func (sk ScopedKeeper) ClaimCapability(ctx sdk.Context, cap *types.Capability, n
|
||||||
// previously claimed or created. After releasing the capability, if no more
|
// previously claimed or created. After releasing the capability, if no more
|
||||||
// owners exist, the capability will be globally removed.
|
// owners exist, the capability will be globally removed.
|
||||||
func (sk ScopedKeeper) ReleaseCapability(ctx sdk.Context, cap *types.Capability) error {
|
func (sk ScopedKeeper) ReleaseCapability(ctx sdk.Context, cap *types.Capability) error {
|
||||||
|
if cap == nil {
|
||||||
|
return sdkerrors.Wrap(types.ErrNilCapability, "cannot release nil capability")
|
||||||
|
}
|
||||||
name := sk.GetCapabilityName(ctx, cap)
|
name := sk.GetCapabilityName(ctx, cap)
|
||||||
if len(name) == 0 {
|
if len(name) == 0 {
|
||||||
return sdkerrors.Wrap(types.ErrCapabilityNotOwned, sk.module)
|
return sdkerrors.Wrap(types.ErrCapabilityNotOwned, sk.module)
|
||||||
|
@ -321,6 +342,9 @@ func (sk ScopedKeeper) ReleaseCapability(ctx sdk.Context, cap *types.Capability)
|
||||||
// by name. The module is not allowed to retrieve capabilities which it does not
|
// by name. The module is not allowed to retrieve capabilities which it does not
|
||||||
// own.
|
// own.
|
||||||
func (sk ScopedKeeper) GetCapability(ctx sdk.Context, name string) (*types.Capability, bool) {
|
func (sk ScopedKeeper) GetCapability(ctx sdk.Context, name string) (*types.Capability, bool) {
|
||||||
|
if strings.TrimSpace(name) == "" {
|
||||||
|
return nil, false
|
||||||
|
}
|
||||||
memStore := ctx.KVStore(sk.memKey)
|
memStore := ctx.KVStore(sk.memKey)
|
||||||
|
|
||||||
key := types.RevCapabilityKey(sk.module, name)
|
key := types.RevCapabilityKey(sk.module, name)
|
||||||
|
@ -332,15 +356,14 @@ func (sk ScopedKeeper) GetCapability(ctx sdk.Context, name string) (*types.Capab
|
||||||
// to still have the capability in the go map since changes to
|
// to still have the capability in the go map since changes to
|
||||||
// go map do not automatically get reverted on tx failure,
|
// go map do not automatically get reverted on tx failure,
|
||||||
// so we delete here to remove unnecessary values in map
|
// so we delete here to remove unnecessary values in map
|
||||||
delete(sk.capMap, index)
|
// TODO: Delete index correctly from capMap by storing some reverse lookup
|
||||||
|
// in-memory map. Issue: https://github.com/cosmos/cosmos-sdk/issues/7805
|
||||||
return nil, false
|
return nil, false
|
||||||
}
|
}
|
||||||
|
|
||||||
cap := sk.capMap[index]
|
cap := sk.capMap[index]
|
||||||
if cap == nil {
|
if cap == nil {
|
||||||
// delete key from store to remove unnecessary mapping
|
panic("capability found in memstore is missing from map")
|
||||||
memStore.Delete(key)
|
|
||||||
return nil, false
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return cap, true
|
return cap, true
|
||||||
|
@ -349,14 +372,20 @@ func (sk ScopedKeeper) GetCapability(ctx sdk.Context, name string) (*types.Capab
|
||||||
// GetCapabilityName allows a module to retrieve the name under which it stored a given
|
// GetCapabilityName allows a module to retrieve the name under which it stored a given
|
||||||
// capability given the capability
|
// capability given the capability
|
||||||
func (sk ScopedKeeper) GetCapabilityName(ctx sdk.Context, cap *types.Capability) string {
|
func (sk ScopedKeeper) GetCapabilityName(ctx sdk.Context, cap *types.Capability) string {
|
||||||
|
if cap == nil {
|
||||||
|
return ""
|
||||||
|
}
|
||||||
memStore := ctx.KVStore(sk.memKey)
|
memStore := ctx.KVStore(sk.memKey)
|
||||||
|
|
||||||
return string(memStore.Get(types.FwdCapabilityKey(sk.module, cap)))
|
return string(memStore.Get(types.FwdCapabilityKey(sk.module, cap)))
|
||||||
}
|
}
|
||||||
|
|
||||||
// Get all the Owners that own the capability associated with the name this ScopedKeeper uses
|
// GetOwners all the Owners that own the capability associated with the name this ScopedKeeper uses
|
||||||
// to refer to the capability
|
// to refer to the capability
|
||||||
func (sk ScopedKeeper) GetOwners(ctx sdk.Context, name string) (*types.CapabilityOwners, bool) {
|
func (sk ScopedKeeper) GetOwners(ctx sdk.Context, name string) (*types.CapabilityOwners, bool) {
|
||||||
|
if strings.TrimSpace(name) == "" {
|
||||||
|
return nil, false
|
||||||
|
}
|
||||||
cap, ok := sk.GetCapability(ctx, name)
|
cap, ok := sk.GetCapability(ctx, name)
|
||||||
if !ok {
|
if !ok {
|
||||||
return nil, false
|
return nil, false
|
||||||
|
@ -382,6 +411,9 @@ func (sk ScopedKeeper) GetOwners(ctx sdk.Context, name string) (*types.Capabilit
|
||||||
// The method returns an error if either the capability or the owners cannot be
|
// The method returns an error if either the capability or the owners cannot be
|
||||||
// retreived from the memstore.
|
// retreived from the memstore.
|
||||||
func (sk ScopedKeeper) LookupModules(ctx sdk.Context, name string) ([]string, *types.Capability, error) {
|
func (sk ScopedKeeper) LookupModules(ctx sdk.Context, name string) ([]string, *types.Capability, error) {
|
||||||
|
if strings.TrimSpace(name) == "" {
|
||||||
|
return nil, nil, sdkerrors.Wrap(types.ErrInvalidCapabilityName, "cannot lookup modules with empty capability name")
|
||||||
|
}
|
||||||
cap, ok := sk.GetCapability(ctx, name)
|
cap, ok := sk.GetCapability(ctx, name)
|
||||||
if !ok {
|
if !ok {
|
||||||
return nil, nil, sdkerrors.Wrap(types.ErrCapabilityNotFound, name)
|
return nil, nil, sdkerrors.Wrap(types.ErrCapabilityNotFound, name)
|
||||||
|
|
|
@ -38,6 +38,9 @@ func (suite *KeeperTestSuite) SetupTest() {
|
||||||
|
|
||||||
func (suite *KeeperTestSuite) TestInitializeAndSeal() {
|
func (suite *KeeperTestSuite) TestInitializeAndSeal() {
|
||||||
sk := suite.keeper.ScopeToModule(banktypes.ModuleName)
|
sk := suite.keeper.ScopeToModule(banktypes.ModuleName)
|
||||||
|
suite.Require().Panics(func() {
|
||||||
|
suite.keeper.ScopeToModule(" ")
|
||||||
|
})
|
||||||
|
|
||||||
caps := make([]*types.Capability, 5)
|
caps := make([]*types.Capability, 5)
|
||||||
// Get Latest Index before creating new ones to sychronize indices correctly
|
// Get Latest Index before creating new ones to sychronize indices correctly
|
||||||
|
@ -105,6 +108,10 @@ func (suite *KeeperTestSuite) TestNewCapability() {
|
||||||
suite.Require().True(ok)
|
suite.Require().True(ok)
|
||||||
suite.Require().Equal(cap, got)
|
suite.Require().Equal(cap, got)
|
||||||
suite.Require().True(cap == got, "expected memory addresses to be equal")
|
suite.Require().True(cap == got, "expected memory addresses to be equal")
|
||||||
|
|
||||||
|
cap, err = sk.NewCapability(suite.ctx, " ")
|
||||||
|
suite.Require().Error(err)
|
||||||
|
suite.Require().Nil(cap)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (suite *KeeperTestSuite) TestOriginalCapabilityKeeper() {
|
func (suite *KeeperTestSuite) TestOriginalCapabilityKeeper() {
|
||||||
|
@ -151,11 +158,15 @@ func (suite *KeeperTestSuite) TestAuthenticateCapability() {
|
||||||
badCap := types.NewCapability(100)
|
badCap := types.NewCapability(100)
|
||||||
suite.Require().False(sk1.AuthenticateCapability(suite.ctx, badCap, "transfer"))
|
suite.Require().False(sk1.AuthenticateCapability(suite.ctx, badCap, "transfer"))
|
||||||
suite.Require().False(sk2.AuthenticateCapability(suite.ctx, badCap, "bond"))
|
suite.Require().False(sk2.AuthenticateCapability(suite.ctx, badCap, "bond"))
|
||||||
|
|
||||||
|
suite.Require().False(sk1.AuthenticateCapability(suite.ctx, cap1, " "))
|
||||||
|
suite.Require().False(sk1.AuthenticateCapability(suite.ctx, nil, "transfer"))
|
||||||
}
|
}
|
||||||
|
|
||||||
func (suite *KeeperTestSuite) TestClaimCapability() {
|
func (suite *KeeperTestSuite) TestClaimCapability() {
|
||||||
sk1 := suite.keeper.ScopeToModule(banktypes.ModuleName)
|
sk1 := suite.keeper.ScopeToModule(banktypes.ModuleName)
|
||||||
sk2 := suite.keeper.ScopeToModule(stakingtypes.ModuleName)
|
sk2 := suite.keeper.ScopeToModule(stakingtypes.ModuleName)
|
||||||
|
sk3 := suite.keeper.ScopeToModule("foo")
|
||||||
|
|
||||||
cap, err := sk1.NewCapability(suite.ctx, "transfer")
|
cap, err := sk1.NewCapability(suite.ctx, "transfer")
|
||||||
suite.Require().NoError(err)
|
suite.Require().NoError(err)
|
||||||
|
@ -171,6 +182,9 @@ func (suite *KeeperTestSuite) TestClaimCapability() {
|
||||||
got, ok = sk2.GetCapability(suite.ctx, "transfer")
|
got, ok = sk2.GetCapability(suite.ctx, "transfer")
|
||||||
suite.Require().True(ok)
|
suite.Require().True(ok)
|
||||||
suite.Require().Equal(cap, got)
|
suite.Require().Equal(cap, got)
|
||||||
|
|
||||||
|
suite.Require().Error(sk3.ClaimCapability(suite.ctx, cap, " "))
|
||||||
|
suite.Require().Error(sk3.ClaimCapability(suite.ctx, nil, "transfer"))
|
||||||
}
|
}
|
||||||
|
|
||||||
func (suite *KeeperTestSuite) TestGetOwners() {
|
func (suite *KeeperTestSuite) TestGetOwners() {
|
||||||
|
@ -237,6 +251,8 @@ func (suite *KeeperTestSuite) TestGetOwners() {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
_, ok := sk1.GetOwners(suite.ctx, " ")
|
||||||
|
suite.Require().False(ok, "got owners from empty capability name")
|
||||||
}
|
}
|
||||||
|
|
||||||
func (suite *KeeperTestSuite) TestReleaseCapability() {
|
func (suite *KeeperTestSuite) TestReleaseCapability() {
|
||||||
|
@ -264,6 +280,8 @@ func (suite *KeeperTestSuite) TestReleaseCapability() {
|
||||||
got, ok = sk1.GetCapability(suite.ctx, "transfer")
|
got, ok = sk1.GetCapability(suite.ctx, "transfer")
|
||||||
suite.Require().False(ok)
|
suite.Require().False(ok)
|
||||||
suite.Require().Nil(got)
|
suite.Require().Nil(got)
|
||||||
|
|
||||||
|
suite.Require().Error(sk1.ReleaseCapability(suite.ctx, nil))
|
||||||
}
|
}
|
||||||
|
|
||||||
func (suite KeeperTestSuite) TestRevertCapability() {
|
func (suite KeeperTestSuite) TestRevertCapability() {
|
||||||
|
|
|
@ -8,9 +8,11 @@ import (
|
||||||
|
|
||||||
// x/capability module sentinel errors
|
// x/capability module sentinel errors
|
||||||
var (
|
var (
|
||||||
ErrCapabilityTaken = sdkerrors.Register(ModuleName, 2, "capability name already taken")
|
ErrInvalidCapabilityName = sdkerrors.Register(ModuleName, 2, "capability name not valid")
|
||||||
ErrOwnerClaimed = sdkerrors.Register(ModuleName, 3, "given owner already claimed capability")
|
ErrNilCapability = sdkerrors.Register(ModuleName, 3, "provided capability is nil")
|
||||||
ErrCapabilityNotOwned = sdkerrors.Register(ModuleName, 4, "capability not owned by module")
|
ErrCapabilityTaken = sdkerrors.Register(ModuleName, 4, "capability name already taken")
|
||||||
ErrCapabilityNotFound = sdkerrors.Register(ModuleName, 5, "capability not found")
|
ErrOwnerClaimed = sdkerrors.Register(ModuleName, 5, "given owner already claimed capability")
|
||||||
ErrCapabilityOwnersNotFound = sdkerrors.Register(ModuleName, 6, "owners not found for capability")
|
ErrCapabilityNotOwned = sdkerrors.Register(ModuleName, 6, "capability not owned by module")
|
||||||
|
ErrCapabilityNotFound = sdkerrors.Register(ModuleName, 7, "capability not found")
|
||||||
|
ErrCapabilityOwnersNotFound = sdkerrors.Register(ModuleName, 8, "owners not found for capability")
|
||||||
)
|
)
|
||||||
|
|
|
@ -108,7 +108,7 @@ func checkMisbehaviourHeader(
|
||||||
chainID, _ = clienttypes.SetVersionNumber(chainID, header.GetHeight().GetVersionNumber())
|
chainID, _ = clienttypes.SetVersionNumber(chainID, header.GetHeight().GetVersionNumber())
|
||||||
}
|
}
|
||||||
|
|
||||||
// - ValidatorSet must have 2/3 similarity with trusted FromValidatorSet
|
// - ValidatorSet must have TrustLevel similarity with trusted FromValidatorSet
|
||||||
// - ValidatorSets on both headers are valid given the last trusted ValidatorSet
|
// - ValidatorSets on both headers are valid given the last trusted ValidatorSet
|
||||||
if err := tmTrustedValset.VerifyCommitLightTrusting(
|
if err := tmTrustedValset.VerifyCommitLightTrusting(
|
||||||
chainID, tmCommit, clientState.TrustLevel.ToTendermint(),
|
chainID, tmCommit, clientState.TrustLevel.ToTendermint(),
|
||||||
|
|
Loading…
Reference in New Issue