From 63ff150aa08bbd6eeafb51f9803c8cafc7cc4a1a Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Fri, 23 Sep 2022 20:19:30 -0700 Subject: [PATCH] fix: all: remove map iteration non-determinism with keys + sorting Fixes some errors flagged by cosmos/gosec like: * https://github.com/cosmos/cosmos-sdk/security/code-scanning/760 * https://github.com/cosmos/cosmos-sdk/security/code-scanning/765 * https://github.com/cosmos/cosmos-sdk/security/code-scanning/767 * https://github.com/cosmos/cosmos-sdk/security/code-scanning/774 * https://github.com/cosmos/cosmos-sdk/security/code-scanning/776 * https://github.com/cosmos/cosmos-sdk/security/code-scanning/815 --- baseapp/baseapp.go | 7 ++++++- orm/model/ormdb/json.go | 8 +++++++- store/rootmulti/store.go | 15 ++++++++++++++- types/coin.go | 2 +- types/module/module.go | 4 +++- x/group/keeper/invariants.go | 10 +++++++++- 6 files changed, 40 insertions(+), 6 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index dd020e8ec022..deeb4fc9e72d 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -2,6 +2,7 @@ package baseapp import ( "fmt" + "sort" "strings" abci "github.com/tendermint/tendermint/abci/types" @@ -9,6 +10,7 @@ import ( "github.com/tendermint/tendermint/libs/log" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" dbm "github.com/tendermint/tm-db" + "golang.org/x/exp/maps" "github.com/cosmos/gogoproto/proto" @@ -251,7 +253,10 @@ func (app *BaseApp) MountTransientStores(keys map[string]*storetypes.TransientSt // MountMemoryStores mounts all in-memory KVStores with the BaseApp's internal // commit multi-store. func (app *BaseApp) MountMemoryStores(keys map[string]*storetypes.MemoryStoreKey) { - for _, memKey := range keys { + skeys := maps.Keys(keys) + sort.Strings(skeys) + for _, key := range skeys { + memKey := keys[key] app.MountStore(memKey, storetypes.StoreTypeMemory) } } diff --git a/orm/model/ormdb/json.go b/orm/model/ormdb/json.go index 7e5ba32160a6..95151a5e7830 100644 --- a/orm/model/ormdb/json.go +++ b/orm/model/ormdb/json.go @@ -42,7 +42,12 @@ func (m moduleDB) DefaultJSON(target ormjson.WriteTarget) error { func (m moduleDB) ValidateJSON(source ormjson.ReadSource) error { errMap := map[protoreflect.FullName]error{} - for name, table := range m.tablesByName { + names := maps.Keys(m.tablesByName) + sort.Slice(names, func(i, j int) bool { + ti, tj := names[i], names[j] + return ti.Name() < tj.Name() + }) + for _, name := range names { r, err := source.OpenReader(name) if err != nil { return err @@ -52,6 +57,7 @@ func (m moduleDB) ValidateJSON(source ormjson.ReadSource) error { continue } + table := m.tablesByName[name] err = table.ValidateJSON(r) if err != nil { errMap[name] = err diff --git a/store/rootmulti/store.go b/store/rootmulti/store.go index de6c453bf9a3..0f652d24bab5 100644 --- a/store/rootmulti/store.go +++ b/store/rootmulti/store.go @@ -37,6 +37,18 @@ const ( const iavlDisablefastNodeDefault = false +func keysForStoreKeyMap[V any](m map[types.StoreKey]V) []types.StoreKey { + keys := make([]types.StoreKey, 0, len(m)) + for key := range m { + keys = append(keys, key) + } + sort.Slice(keys, func(i, j int) bool { + ki, kj := keys[i], keys[j] + return ki.Name() < kj.Name() + }) + return keys +} + // Store is composed of many CommitStores. Name contrasts with // cacheMultiStore which is used for branching other MultiStores. It implements // the CommitMultiStore interface. @@ -718,7 +730,8 @@ func (rs *Store) Snapshot(height uint64, protoWriter protoio.Writer) error { name string } stores := []namedStore{} - for key := range rs.stores { + keys := keysForStoreKeyMap(rs.stores) + for _, key := range keys { switch store := rs.GetCommitKVStore(key).(type) { case *iavl.Store: stores = append(stores, namedStore{name: key.Name(), Store: store}) diff --git a/types/coin.go b/types/coin.go index 68d604105ca5..cf86af8c039a 100644 --- a/types/coin.go +++ b/types/coin.go @@ -337,7 +337,7 @@ func (coins Coins) safeAdd(coinsB Coins) (coalesced Coins) { } } - for denom, cL := range uniqCoins { + for denom, cL := range uniqCoins { //#nosec comboCoin := Coin{Denom: denom, Amount: NewInt(0)} for _, c := range cL { comboCoin = comboCoin.Add(c) diff --git a/types/module/module.go b/types/module/module.go index 2828373372cd..b03bb4c4f7c7 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -339,13 +339,15 @@ func (m *Manager) assertNoForgottenModules(setOrderFnName string, moduleNames [] for _, m := range moduleNames { ms[m] = true } + allKeys := maps.Keys(m.Modules) var missing []string - for m := range m.Modules { + for _, m := range allKeys { if !ms[m] { missing = append(missing, m) } } if len(missing) != 0 { + sort.Strings(missing) panic(fmt.Sprintf( "%s: all modules must be defined when setting %s, missing: %v", setOrderFnName, setOrderFnName, missing)) } diff --git a/x/group/keeper/invariants.go b/x/group/keeper/invariants.go index 24b592a37020..087691473f05 100644 --- a/x/group/keeper/invariants.go +++ b/x/group/keeper/invariants.go @@ -3,6 +3,9 @@ package keeper import ( "fmt" "math" + "sort" + + "golang.org/x/exp/maps" storetypes "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -53,7 +56,12 @@ func GroupTotalWeightInvariantHelper(ctx sdk.Context, key storetypes.StoreKey, g groups[groupInfo.Id] = groupInfo } - for _, groupInfo := range groups { + groupByIDs := maps.Keys(groups) + sort.Slice(groupByIDs, func(i, j int) bool { + return groupByIDs[i] < groupByIDs[j] + }) + for _, groupID := range groupByIDs { + groupInfo := groups[groupID] membersWeight, err := groupmath.NewNonNegativeDecFromString("0") if err != nil { msg += fmt.Sprintf("error while parsing positive dec zero for group member\n%v\n", err)