From 8ad5585c9e0772ba63c75d60a0cf8eb92417da3a Mon Sep 17 00:00:00 2001 From: Jaeseung Lee <41176085+tkxkd0159@users.noreply.github.com> Date: Wed, 8 May 2024 16:59:32 +0900 Subject: [PATCH] fix: fbridge auth checking bug (#1361) --- CHANGELOG.md | 1 + x/fbridge/keeper/keeper.go | 5 +++ x/fbridge/keeper/keeper_test.go | 64 +++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+) create mode 100644 x/fbridge/keeper/keeper_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 146a25d1a8..09ebbbc7d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (x/auth) [#1319](https://github.com/Finschia/finschia-sdk/pull/1319) prevent signing from wrong key in multisig * (x/mint, x/slashing) [\#1323](https://github.com/Finschia/finschia-sdk/pull/1323) add missing nil check for params validation * (x/server) [\#1337](https://github.com/Finschia/finschia-sdk/pull/1337) fix panic when defining minimum gas config as `100stake;100uatom`. Use a `,` delimiter instead of `;`. Fixes the server config getter to use the correct delimiter (backport cosmos/cosmos-sdk#18537) +* (x/fbridge) [\#1361](https://github.com/Finschia/finschia-sdk/pull/1361) Fixes fbridge auth checking bug ### Removed diff --git a/x/fbridge/keeper/keeper.go b/x/fbridge/keeper/keeper.go index 18cc8e0ef9..a226a1809e 100644 --- a/x/fbridge/keeper/keeper.go +++ b/x/fbridge/keeper/keeper.go @@ -37,10 +37,15 @@ func NewKeeper( panic(errors.New("fbridge module account has not been set")) } + found := false for _, addr := range types.AuthorityCandiates() { if authority == addr.String() { + found = true break } + } + + if !found { panic("x/bridge authority must be the gov or foundation module account") } diff --git a/x/fbridge/keeper/keeper_test.go b/x/fbridge/keeper/keeper_test.go new file mode 100644 index 0000000000..f1e5891058 --- /dev/null +++ b/x/fbridge/keeper/keeper_test.go @@ -0,0 +1,64 @@ +package keeper_test + +import ( + "testing" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" + + authtypes "github.com/Finschia/finschia-sdk/x/auth/types" + "github.com/Finschia/finschia-sdk/x/fbridge/keeper" + "github.com/Finschia/finschia-sdk/x/fbridge/testutil" + "github.com/Finschia/finschia-sdk/x/fbridge/types" + "github.com/Finschia/finschia-sdk/x/foundation" + govtypes "github.com/Finschia/finschia-sdk/x/gov/types" +) + +func TestNewKeeper(t *testing.T) { + key, memKey, _, encCfg, _, bankKeeper, _ := testutil.PrepareFbridgeTest(t, 0) + authKeeper := testutil.NewMockAccountKeeper(gomock.NewController(t)) + + tcs := map[string]struct { + malleate func() + isPanic bool + }{ + "fbridge module account has not been set": { + malleate: func() { + authKeeper.EXPECT().GetModuleAddress(types.ModuleName).Return(nil).Times(1) + keeper.NewKeeper(encCfg.Codec, key, memKey, authKeeper, bankKeeper, "stake", types.DefaultAuthority().String()) + }, + isPanic: true, + }, + "fbridge authority must be the gov or foundation module account": { + malleate: func() { + authKeeper.EXPECT().GetModuleAddress(types.ModuleName).Return(authtypes.NewModuleAddress(types.ModuleName)).Times(1) + keeper.NewKeeper(encCfg.Codec, key, memKey, authKeeper, bankKeeper, "stake", authtypes.NewModuleAddress("invalid").String()) + }, + isPanic: true, + }, + "success - gov authority": { + malleate: func() { + authKeeper.EXPECT().GetModuleAddress(types.ModuleName).Return(authtypes.NewModuleAddress(types.ModuleName)).Times(1) + keeper.NewKeeper(encCfg.Codec, key, memKey, authKeeper, bankKeeper, "stake", authtypes.NewModuleAddress(govtypes.ModuleName).String()) + }, + isPanic: false, + }, + "success - foundation authority": { + malleate: func() { + authKeeper.EXPECT().GetModuleAddress(types.ModuleName).Return(authtypes.NewModuleAddress(types.ModuleName)).Times(1) + keeper.NewKeeper(encCfg.Codec, key, memKey, authKeeper, bankKeeper, "stake", authtypes.NewModuleAddress(foundation.ModuleName).String()) + }, + isPanic: false, + }, + } + + for name, tc := range tcs { + t.Run(name, func(t *testing.T) { + if tc.isPanic { + require.Panics(t, tc.malleate) + } else { + tc.malleate() + } + }) + } +}