From fcb9d84edbec2e3bfa5d357bb100dbf569d06e07 Mon Sep 17 00:00:00 2001
From: Julien Robert <julien@rbrt.fr>
Date: Sun, 21 Apr 2024 21:27:56 +0200
Subject: [PATCH] fix(x/authz,x/feegrant): check blocked address (#20102)

---
 x/authz/expected_keepers.go                   |  1 +
 x/authz/keeper/keeper.go                      |  8 ++++++++
 x/authz/keeper/keeper_test.go                 |  4 ++--
 x/authz/keeper/msg_server.go                  |  4 ++++
 x/authz/module/module.go                      |  2 +-
 x/authz/testutil/expected_keepers_mocks.go    | 14 ++++++++++++++
 x/feegrant/expected_keepers.go                |  1 +
 x/feegrant/keeper/keeper.go                   | 12 ++++++++++++
 x/feegrant/keeper/keeper_test.go              |  6 ++++--
 x/feegrant/module/module.go                   |  2 +-
 x/feegrant/testutil/expected_keepers_mocks.go | 14 ++++++++++++++
 11 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/x/authz/expected_keepers.go b/x/authz/expected_keepers.go
index 2db295aa7e52..c59837d2fc56 100644
--- a/x/authz/expected_keepers.go
+++ b/x/authz/expected_keepers.go
@@ -20,4 +20,5 @@ type AccountKeeper interface {
 type BankKeeper interface {
 	SpendableCoins(ctx context.Context, addr sdk.AccAddress) sdk.Coins
 	IsSendEnabledCoins(ctx context.Context, coins ...sdk.Coin) error
+	BlockedAddr(addr sdk.AccAddress) bool
 }
diff --git a/x/authz/keeper/keeper.go b/x/authz/keeper/keeper.go
index c7dc7dec5421..974c18000e42 100644
--- a/x/authz/keeper/keeper.go
+++ b/x/authz/keeper/keeper.go
@@ -34,6 +34,7 @@ type Keeper struct {
 	cdc          codec.Codec
 	router       baseapp.MessageRouter
 	authKeeper   authz.AccountKeeper
+	bankKeeper   authz.BankKeeper
 }
 
 // NewKeeper constructs a message authorization Keeper
@@ -46,6 +47,13 @@ func NewKeeper(storeService corestoretypes.KVStoreService, cdc codec.Codec, rout
 	}
 }
 
+// Super ugly hack to not be breaking in v0.50 and v0.47
+// DO NOT USE.
+func (k Keeper) SetBankKeeper(bk authz.BankKeeper) Keeper {
+	k.bankKeeper = bk
+	return k
+}
+
 // Logger returns a module-specific logger.
 func (k Keeper) Logger(ctx context.Context) log.Logger {
 	sdkCtx := sdk.UnwrapSDKContext(ctx)
diff --git a/x/authz/keeper/keeper_test.go b/x/authz/keeper/keeper_test.go
index 066e7d70750a..94f3f50fa3c0 100644
--- a/x/authz/keeper/keeper_test.go
+++ b/x/authz/keeper/keeper_test.go
@@ -68,14 +68,14 @@ func (s *TestSuite) SetupTest() {
 	// gomock initializations
 	ctrl := gomock.NewController(s.T())
 	s.accountKeeper = authztestutil.NewMockAccountKeeper(ctrl)
-
 	s.accountKeeper.EXPECT().AddressCodec().Return(address.NewBech32Codec("cosmos")).AnyTimes()
 
 	s.bankKeeper = authztestutil.NewMockBankKeeper(ctrl)
 	banktypes.RegisterInterfaces(s.encCfg.InterfaceRegistry)
 	banktypes.RegisterMsgServer(s.baseApp.MsgServiceRouter(), s.bankKeeper)
+	s.bankKeeper.EXPECT().BlockedAddr(gomock.Any()).Return(false).AnyTimes()
 
-	s.authzKeeper = authzkeeper.NewKeeper(storeService, s.encCfg.Codec, s.baseApp.MsgServiceRouter(), s.accountKeeper)
+	s.authzKeeper = authzkeeper.NewKeeper(storeService, s.encCfg.Codec, s.baseApp.MsgServiceRouter(), s.accountKeeper).SetBankKeeper(s.bankKeeper)
 
 	queryHelper := baseapp.NewQueryServerTestHelper(s.ctx, s.encCfg.InterfaceRegistry)
 	authz.RegisterQueryServer(queryHelper, s.authzKeeper)
diff --git a/x/authz/keeper/msg_server.go b/x/authz/keeper/msg_server.go
index b6755a9f8436..98e24a44f2a2 100644
--- a/x/authz/keeper/msg_server.go
+++ b/x/authz/keeper/msg_server.go
@@ -38,6 +38,10 @@ func (k Keeper) Grant(goCtx context.Context, msg *authz.MsgGrant) (*authz.MsgGra
 	ctx := sdk.UnwrapSDKContext(goCtx)
 	granteeAcc := k.authKeeper.GetAccount(ctx, grantee)
 	if granteeAcc == nil {
+		if k.bankKeeper.BlockedAddr(grantee) {
+			return nil, sdkerrors.ErrUnauthorized.Wrapf("%s is not allowed to receive funds", grantee)
+		}
+
 		granteeAcc = k.authKeeper.NewAccountWithAddress(ctx, grantee)
 		k.authKeeper.SetAccount(ctx, granteeAcc)
 	}
diff --git a/x/authz/module/module.go b/x/authz/module/module.go
index 248a75b8584b..3546f70e9d8e 100644
--- a/x/authz/module/module.go
+++ b/x/authz/module/module.go
@@ -113,7 +113,7 @@ type AppModule struct {
 func NewAppModule(cdc codec.Codec, keeper keeper.Keeper, ak authz.AccountKeeper, bk authz.BankKeeper, registry cdctypes.InterfaceRegistry) AppModule {
 	return AppModule{
 		AppModuleBasic: AppModuleBasic{cdc: cdc, ac: ak.AddressCodec()},
-		keeper:         keeper,
+		keeper:         keeper.SetBankKeeper(bk), // Super ugly hack to not be api breaking in v0.50 and v0.47
 		accountKeeper:  ak,
 		bankKeeper:     bk,
 		registry:       registry,
diff --git a/x/authz/testutil/expected_keepers_mocks.go b/x/authz/testutil/expected_keepers_mocks.go
index b9e8f521c327..d751684835dd 100644
--- a/x/authz/testutil/expected_keepers_mocks.go
+++ b/x/authz/testutil/expected_keepers_mocks.go
@@ -113,6 +113,20 @@ func (m *MockBankKeeper) EXPECT() *MockBankKeeperMockRecorder {
 	return m.recorder
 }
 
+// BlockedAddr mocks base method.
+func (m *MockBankKeeper) BlockedAddr(addr types.AccAddress) bool {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "BlockedAddr", addr)
+	ret0, _ := ret[0].(bool)
+	return ret0
+}
+
+// BlockedAddr indicates an expected call of BlockedAddr.
+func (mr *MockBankKeeperMockRecorder) BlockedAddr(addr interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BlockedAddr", reflect.TypeOf((*MockBankKeeper)(nil).BlockedAddr), addr)
+}
+
 // IsSendEnabledCoins mocks base method.
 func (m *MockBankKeeper) IsSendEnabledCoins(ctx context.Context, coins ...types.Coin) error {
 	m.ctrl.T.Helper()
diff --git a/x/feegrant/expected_keepers.go b/x/feegrant/expected_keepers.go
index ce6a2d0a5698..e09da85fc080 100644
--- a/x/feegrant/expected_keepers.go
+++ b/x/feegrant/expected_keepers.go
@@ -24,4 +24,5 @@ type AccountKeeper interface {
 type BankKeeper interface {
 	SpendableCoins(ctx context.Context, addr sdk.AccAddress) sdk.Coins
 	SendCoinsFromAccountToModule(ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error
+	BlockedAddr(addr sdk.AccAddress) bool
 }
diff --git a/x/feegrant/keeper/keeper.go b/x/feegrant/keeper/keeper.go
index 37f46d97a215..02ec88b69723 100644
--- a/x/feegrant/keeper/keeper.go
+++ b/x/feegrant/keeper/keeper.go
@@ -24,6 +24,7 @@ type Keeper struct {
 	cdc          codec.BinaryCodec
 	storeService store.KVStoreService
 	authKeeper   feegrant.AccountKeeper
+	bankKeeper   feegrant.BankKeeper
 }
 
 var _ ante.FeegrantKeeper = &Keeper{}
@@ -37,6 +38,13 @@ func NewKeeper(cdc codec.BinaryCodec, storeService store.KVStoreService, ak feeg
 	}
 }
 
+// Super ugly hack to not be breaking in v0.50 and v0.47
+// DO NOT USE.
+func (k Keeper) SetBankKeeper(bk feegrant.BankKeeper) Keeper {
+	k.bankKeeper = bk
+	return k
+}
+
 // Logger returns a module-specific logger.
 func (k Keeper) Logger(ctx sdk.Context) log.Logger {
 	return ctx.Logger().With("module", fmt.Sprintf("x/%s", feegrant.ModuleName))
@@ -52,6 +60,10 @@ func (k Keeper) GrantAllowance(ctx context.Context, granter, grantee sdk.AccAddr
 	// create the account if it is not in account state
 	granteeAcc := k.authKeeper.GetAccount(ctx, grantee)
 	if granteeAcc == nil {
+		if k.bankKeeper.BlockedAddr(grantee) {
+			return errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to receive funds", grantee)
+		}
+
 		granteeAcc = k.authKeeper.NewAccountWithAddress(ctx, grantee)
 		k.authKeeper.SetAccount(ctx, granteeAcc)
 	}
diff --git a/x/feegrant/keeper/keeper_test.go b/x/feegrant/keeper/keeper_test.go
index fcfb5df2a5d5..fb7f661b217a 100644
--- a/x/feegrant/keeper/keeper_test.go
+++ b/x/feegrant/keeper/keeper_test.go
@@ -31,6 +31,7 @@ type KeeperTestSuite struct {
 	atom           sdk.Coins
 	feegrantKeeper keeper.Keeper
 	accountKeeper  *feegranttestutil.MockAccountKeeper
+	bankKeeper     *feegranttestutil.MockBankKeeper
 }
 
 func TestKeeperTestSuite(t *testing.T) {
@@ -49,10 +50,11 @@ func (suite *KeeperTestSuite) SetupTest() {
 	for i := 0; i < len(suite.addrs); i++ {
 		suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[i]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[i])).AnyTimes()
 	}
-
 	suite.accountKeeper.EXPECT().AddressCodec().Return(codecaddress.NewBech32Codec("cosmos")).AnyTimes()
+	suite.bankKeeper = feegranttestutil.NewMockBankKeeper(ctrl)
+	suite.bankKeeper.EXPECT().BlockedAddr(gomock.Any()).Return(false).AnyTimes()
 
-	suite.feegrantKeeper = keeper.NewKeeper(encCfg.Codec, runtime.NewKVStoreService(key), suite.accountKeeper)
+	suite.feegrantKeeper = keeper.NewKeeper(encCfg.Codec, runtime.NewKVStoreService(key), suite.accountKeeper).SetBankKeeper(suite.bankKeeper)
 	suite.ctx = testCtx.Ctx
 	suite.msgSrvr = keeper.NewMsgServerImpl(suite.feegrantKeeper)
 	suite.atom = sdk.NewCoins(sdk.NewCoin("atom", sdkmath.NewInt(555)))
diff --git a/x/feegrant/module/module.go b/x/feegrant/module/module.go
index 23ae96afce70..24805f6e667a 100644
--- a/x/feegrant/module/module.go
+++ b/x/feegrant/module/module.go
@@ -120,7 +120,7 @@ type AppModule struct {
 func NewAppModule(cdc codec.Codec, ak feegrant.AccountKeeper, bk feegrant.BankKeeper, keeper keeper.Keeper, registry cdctypes.InterfaceRegistry) AppModule {
 	return AppModule{
 		AppModuleBasic: AppModuleBasic{cdc: cdc, ac: ak.AddressCodec()},
-		keeper:         keeper,
+		keeper:         keeper.SetBankKeeper(bk),
 		accountKeeper:  ak,
 		bankKeeper:     bk,
 		registry:       registry,
diff --git a/x/feegrant/testutil/expected_keepers_mocks.go b/x/feegrant/testutil/expected_keepers_mocks.go
index d7567b8266d3..319a4d2a265a 100644
--- a/x/feegrant/testutil/expected_keepers_mocks.go
+++ b/x/feegrant/testutil/expected_keepers_mocks.go
@@ -141,6 +141,20 @@ func (m *MockBankKeeper) EXPECT() *MockBankKeeperMockRecorder {
 	return m.recorder
 }
 
+// BlockedAddr mocks base method.
+func (m *MockBankKeeper) BlockedAddr(addr types.AccAddress) bool {
+	m.ctrl.T.Helper()
+	ret := m.ctrl.Call(m, "BlockedAddr", addr)
+	ret0, _ := ret[0].(bool)
+	return ret0
+}
+
+// BlockedAddr indicates an expected call of BlockedAddr.
+func (mr *MockBankKeeperMockRecorder) BlockedAddr(addr interface{}) *gomock.Call {
+	mr.mock.ctrl.T.Helper()
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BlockedAddr", reflect.TypeOf((*MockBankKeeper)(nil).BlockedAddr), addr)
+}
+
 // SendCoinsFromAccountToModule mocks base method.
 func (m *MockBankKeeper) SendCoinsFromAccountToModule(ctx context.Context, senderAddr types.AccAddress, recipientModule string, amt types.Coins) error {
 	m.ctrl.T.Helper()