From 9fc4f4e9e76098b8cef7c3078ff961928488d998 Mon Sep 17 00:00:00 2001 From: "jaeseung.bae" Date: Tue, 12 Mar 2024 18:54:46 +0900 Subject: [PATCH 1/5] fix: add non-zero check of nextTokenID.Id for genesis --- x/collection/keeper/genesis.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/x/collection/keeper/genesis.go b/x/collection/keeper/genesis.go index 01330383cc..0b786c78e6 100644 --- a/x/collection/keeper/genesis.go +++ b/x/collection/keeper/genesis.go @@ -97,6 +97,9 @@ func (k Keeper) InitGenesis(ctx sdk.Context, data *collection.GenesisState) { contractID := contractNextTokenIDs.ContractId for _, nextTokenID := range contractNextTokenIDs.TokenIds { + if nextTokenID.Id.IsZero() { + panic("nextTokenID.Id is not supposed to be zero") + } k.setNextTokenID(ctx, contractID, nextTokenID.ClassId, nextTokenID.Id) } From a0783c909442f0625c84b0c27d47b71fd2ab12e8 Mon Sep 17 00:00:00 2001 From: "jaeseung.bae" Date: Wed, 13 Mar 2024 16:48:25 +0900 Subject: [PATCH 2/5] chore: add testcase --- x/collection/keeper/genesis_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/x/collection/keeper/genesis_test.go b/x/collection/keeper/genesis_test.go index fc6c0c836c..59ec1ffecb 100644 --- a/x/collection/keeper/genesis_test.go +++ b/x/collection/keeper/genesis_test.go @@ -1,6 +1,7 @@ package keeper_test import ( + "github.com/Finschia/finschia-sdk/types" "github.com/Finschia/finschia-sdk/x/collection" ) @@ -28,3 +29,17 @@ func (s *KeeperTestSuite) TestImportExportGenesis() { newGenesis := s.keeper.ExportGenesis(s.ctx) s.Require().Equal(genesis, newGenesis) } + +func (s *KeeperTestSuite) TestShouldPanicWhenZeroNextTokenIdInGenesis() { + dontCare := "12345678" + state := &collection.GenesisState{ + Params: collection.Params{}, + NextTokenIds: []collection.ContractNextTokenIDs{ + {ContractId: dontCare, TokenIds: []collection.NextTokenID{ + {ClassId: dontCare, Id: types.NewUint(0)}, + }}, + }, + } + + s.Require().Panics(func() { s.keeper.InitGenesis(s.ctx, state) }) +} From c65474c7707f0cc9cb786f6ada0cfc05ba73b211 Mon Sep 17 00:00:00 2001 From: "jaeseung.bae" Date: Wed, 13 Mar 2024 18:08:11 +0900 Subject: [PATCH 3/5] chore: update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c5c5569f4..3843499505 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes * chore(deps) [\#1141](https://github.com/Finschia/finschia-sdk/pull/1141) Bump github.com/cosmos/ledger-cosmos-go from 0.12.2 to 0.13.2 to fix ledger signing issue * (x/auth, x/slashing) [\#1179](https://github.com/Finschia/finschia-sdk/pull/1179) modify missing changes of converting to tendermint +* (x/collection) [\#1276](https://github.com/Finschia/finschia-sdk/pull/1276) eliminates potential risk for Insufficient Sanity Check of tokenID in Genesis ### Removed From 90b8a4505add6fdfddc5fea0639833daf68285c0 Mon Sep 17 00:00:00 2001 From: "jaeseung.bae" Date: Fri, 15 Mar 2024 17:21:29 +0900 Subject: [PATCH 4/5] chore: move validation logic to validate function --- x/collection/genesis.go | 7 +++++-- x/collection/keeper/genesis.go | 3 --- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/x/collection/genesis.go b/x/collection/genesis.go index ac08c34fe0..028101b3f7 100644 --- a/x/collection/genesis.go +++ b/x/collection/genesis.go @@ -64,8 +64,11 @@ func ValidateGenesis(data GenesisState) error { if len(contractNextTokenIDs.TokenIds) == 0 { return sdkerrors.ErrInvalidRequest.Wrap("next token ids cannot be empty") } - for _, nextTokenIDs := range contractNextTokenIDs.TokenIds { - if err := ValidateClassID(nextTokenIDs.ClassId); err != nil { + for _, nextTokenID := range contractNextTokenIDs.TokenIds { + if nextTokenID.Id.IsZero() { + return sdkerrors.ErrInvalidRequest.Wrap("nextTokenID.Id is not supposed to be zero") + } + if err := ValidateClassID(nextTokenID.ClassId); err != nil { return err } } diff --git a/x/collection/keeper/genesis.go b/x/collection/keeper/genesis.go index 0b786c78e6..01330383cc 100644 --- a/x/collection/keeper/genesis.go +++ b/x/collection/keeper/genesis.go @@ -97,9 +97,6 @@ func (k Keeper) InitGenesis(ctx sdk.Context, data *collection.GenesisState) { contractID := contractNextTokenIDs.ContractId for _, nextTokenID := range contractNextTokenIDs.TokenIds { - if nextTokenID.Id.IsZero() { - panic("nextTokenID.Id is not supposed to be zero") - } k.setNextTokenID(ctx, contractID, nextTokenID.ClassId, nextTokenID.Id) } From 426d792663924bbdc4391bfcafe20f4f6386fe97 Mon Sep 17 00:00:00 2001 From: "jaeseung.bae" Date: Fri, 15 Mar 2024 17:25:57 +0900 Subject: [PATCH 5/5] chore: add testcase --- x/collection/genesis_test.go | 11 +++++++++++ x/collection/keeper/genesis_test.go | 15 --------------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/x/collection/genesis_test.go b/x/collection/genesis_test.go index d4c7d0a7c1..4fcc543541 100644 --- a/x/collection/genesis_test.go +++ b/x/collection/genesis_test.go @@ -445,6 +445,17 @@ func TestValidateGenesis(t *testing.T) { }, false, }, + "should throw error when next token id is zero in genesis": { + &collection.GenesisState{ + Params: collection.Params{}, + NextTokenIds: []collection.ContractNextTokenIDs{ + {ContractId: "deadbeef", TokenIds: []collection.NextTokenID{ + {ClassId: "deadbeef", Id: sdk.NewUint(0)}, + }}, + }, + }, + false, + }, } for name, tc := range testCases { diff --git a/x/collection/keeper/genesis_test.go b/x/collection/keeper/genesis_test.go index 59ec1ffecb..fc6c0c836c 100644 --- a/x/collection/keeper/genesis_test.go +++ b/x/collection/keeper/genesis_test.go @@ -1,7 +1,6 @@ package keeper_test import ( - "github.com/Finschia/finschia-sdk/types" "github.com/Finschia/finschia-sdk/x/collection" ) @@ -29,17 +28,3 @@ func (s *KeeperTestSuite) TestImportExportGenesis() { newGenesis := s.keeper.ExportGenesis(s.ctx) s.Require().Equal(genesis, newGenesis) } - -func (s *KeeperTestSuite) TestShouldPanicWhenZeroNextTokenIdInGenesis() { - dontCare := "12345678" - state := &collection.GenesisState{ - Params: collection.Params{}, - NextTokenIds: []collection.ContractNextTokenIDs{ - {ContractId: dontCare, TokenIds: []collection.NextTokenID{ - {ClassId: dontCare, Id: types.NewUint(0)}, - }}, - }, - } - - s.Require().Panics(func() { s.keeper.InitGenesis(s.ctx, state) }) -}