From 98164408932ee74369b7ce18aa13bb1a1a6ba76b Mon Sep 17 00:00:00 2001 From: Alex | Interchain Labs Date: Thu, 20 Feb 2025 11:14:24 -0500 Subject: [PATCH] Merge commit from fork * Prevent empty groups (cherry picked from commit 95090a0ec1f193104ba7d6d033a490d1515e54d8) * Handle inflight proposals (cherry picked from commit 855983471882068894ed7952b592ab3e61b464b9) * No empty group with simulations * Update changelog * Set release date * updates * Update RELEASE_NOTES.md * Update RELEASE_NOTES.md --------- Co-authored-by: Alex Peters Co-authored-by: Julien Robert --- CHANGELOG.md | 7 + RELEASE_NOTES.md | 24 +--- x/group/keeper/msg_server.go | 6 +- x/group/keeper/msg_server_test.go | 210 ++++++++++++++---------------- x/group/simulation/operations.go | 2 +- x/group/types.go | 4 +- x/group/types_test.go | 22 ++++ 7 files changed, 139 insertions(+), 136 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d1d0caf379f..3b0b32c089b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,13 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +## [v0.50.12](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.12) - 2025-02-20 + +### Bug Fixes + +* [GHSA-x5vx-95h7-rv4p](https://github.com/cosmos/cosmos-sdk/security/advisories/GHSA-x5vx-95h7-rv4p) Fix Group module can halt chain when handling a malicious proposal + + ## [v0.50.11](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.11) - 2024-12-16 ### Features diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 7eaf4389c4d1..7f45285b4abc 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -1,26 +1,16 @@ -# Cosmos SDK v0.50.11 Release Notes +# Cosmos SDK v0.50.12 Release Notes 💬 [**Release Discussion**](https://github.com/orgs/cosmos/discussions/58) ## 🚀 Highlights -We are back on schedule for our monthly v0.50.x patch releases. -The last two months, next to ramping up on v0.52 and v2, we added a few bug fixes and (UX) improvements. +This patch release fixes [GHSA-x5vx-95h7-rv4p](https://github.com/cosmos/cosmos-sdk/security/advisories/GHSA-x5vx-95h7-rv4p). +It resolves a `x/group` module issue that can halt chain when handling a malicious proposal. +Only users of the `x/group` module are affected by this issue. -Notable changes: - -* Fix [ABS-0043/ABS-0044](https://github.com/cosmos/cosmos-sdk/security/advisories/GHSA-8wcc-m6j2-qxvm). -* New Linux-only backend that adds Linux kernel's `keyctl` support -* Skip sims test when running dry on validators +We recommended to upgrade to this patch release as soon as possible. +When upgrading from <= v0.50.11, please use a chain upgrade to ensure that 2/3 of the validator power upgrade to v0.50.12. ## 📝 Changelog -Check out the [changelog](https://github.com/cosmos/cosmos-sdk/blob/v0.50.11/CHANGELOG.md) for an exhaustive list of changes, or [compare changes](https://github.com/cosmos/cosmos-sdk/compare/v0.50.10...v0.50.11) from the last release. - -## Maintenance Policy - -Cosmos SDK Olympus (v0.52) final release is approaching really soon. That means the Eden line (v0.50.x) will soon only be supported for bug fixes only, as per our release policy. Earlier versions are not maintained. - -Note, that the next SDK release, v0.52, does not include `x/params` migration, when migrating from < v0.47, v0.50.x **or** v0.47.x, is a mandatory migration. - -Start integrating with [Cosmos SDK Eden (v0.52)](https://github.com/cosmos/cosmos-sdk/blob/main/UPGRADING.md#v052x) and enjoy and the new features and performance improvements. +Check out the [changelog](https://github.com/cosmos/cosmos-sdk/blob/v0.50.12/CHANGELOG.md) for an exhaustive list of changes, or [compare changes](https://github.com/cosmos/cosmos-sdk/compare/v0.50.11...v0.50.12) from the last release. diff --git a/x/group/keeper/msg_server.go b/x/group/keeper/msg_server.go index b36215a75e60..85dd52227659 100644 --- a/x/group/keeper/msg_server.go +++ b/x/group/keeper/msg_server.go @@ -200,6 +200,10 @@ func (k Keeper) UpdateGroupMembers(goCtx context.Context, msg *group.MsgUpdateGr return err } } + // ensure that group has one or more members + if totalWeight.IsZero() { + return errorsmod.Wrap(errors.ErrInvalid, "group must not be empty") + } // Update group in the groupTable. g.TotalWeight = totalWeight.String() g.Version++ @@ -1145,10 +1149,8 @@ func (k Keeper) validateMembers(members []group.MemberRequest) error { if _, err := math.NewNonNegativeDecFromString(member.Weight); err != nil { return errorsmod.Wrap(err, "weight must be non negative") } - index[member.Address] = struct{}{} } - return nil } diff --git a/x/group/keeper/msg_server_test.go b/x/group/keeper/msg_server_test.go index 32b058523791..7ca05a766a75 100644 --- a/x/group/keeper/msg_server_test.go +++ b/x/group/keeper/msg_server_test.go @@ -237,20 +237,13 @@ func (s *TestSuite) TestCreateGroup() { } func (s *TestSuite) TestUpdateGroupMembers() { - addrs := s.addrs - addr3 := addrs[2] - addr4 := addrs[3] - addr5 := addrs[4] - addr6 := addrs[5] - - member1 := addr5.String() - member2 := addr6.String() - members := []group.MemberRequest{{ - Address: member1, - Weight: "1", - }} + unknownAddr, myAdmin := s.addrs[0].String(), s.addrs[1].String() + member1, member2, member3 := s.addrs[2].String(), s.addrs[3].String(), s.addrs[4].String() + members := []group.MemberRequest{ + {Address: member1, Weight: "1"}, + {Address: member2, Weight: "2"}, + } - myAdmin := addr4.String() groupRes, err := s.groupKeeper.CreateGroup(s.ctx, &group.MsgCreateGroup{ Admin: myAdmin, Members: members, @@ -270,7 +263,7 @@ func (s *TestSuite) TestUpdateGroupMembers() { GroupId: 0, Admin: myAdmin, MemberUpdates: []group.MemberRequest{{ - Address: member2, + Address: member3, Weight: "2", }}, }, @@ -303,7 +296,7 @@ func (s *TestSuite) TestUpdateGroupMembers() { Admin: myAdmin, MemberUpdates: []group.MemberRequest{ { - Address: member2, + Address: member3, Weight: "2", Metadata: strings.Repeat("a", 256), }, @@ -314,90 +307,77 @@ func (s *TestSuite) TestUpdateGroupMembers() { }, "add new member": { req: &group.MsgUpdateGroupMembers{ - GroupId: groupID, - Admin: myAdmin, - MemberUpdates: []group.MemberRequest{{ - Address: member2, - Weight: "2", - }}, + GroupId: groupID, + Admin: myAdmin, + MemberUpdates: []group.MemberRequest{{Address: member3, Weight: "3"}}, }, expGroup: &group.GroupInfo{ Id: groupID, Admin: myAdmin, - TotalWeight: "3", + TotalWeight: "6", Version: 2, CreatedAt: s.blockTime, }, expMembers: []*group.GroupMember{ { - Member: &group.Member{ - Address: member2, - Weight: "2", - AddedAt: s.sdkCtx.BlockTime(), - }, + Member: &group.Member{Address: member3, Weight: "3", AddedAt: s.blockTime}, GroupId: groupID, }, { - Member: &group.Member{ - Address: member1, - Weight: "1", - AddedAt: s.blockTime, - }, + Member: &group.Member{Address: member1, Weight: "1", AddedAt: s.blockTime}, + GroupId: groupID, + }, + { + Member: &group.Member{Address: member2, Weight: "2", AddedAt: s.blockTime}, GroupId: groupID, }, }, }, "update member": { req: &group.MsgUpdateGroupMembers{ - GroupId: groupID, - Admin: myAdmin, - MemberUpdates: []group.MemberRequest{{ - Address: member1, - Weight: "2", - }}, + GroupId: groupID, + Admin: myAdmin, + MemberUpdates: []group.MemberRequest{{Address: member1, Weight: "2"}}, }, expGroup: &group.GroupInfo{ Id: groupID, Admin: myAdmin, - TotalWeight: "2", + TotalWeight: "4", Version: 2, CreatedAt: s.blockTime, }, expMembers: []*group.GroupMember{ { + Member: &group.Member{Address: member1, Weight: "2", AddedAt: s.blockTime}, + GroupId: groupID, + }, + { + Member: &group.Member{Address: member2, Weight: "2", AddedAt: s.blockTime}, GroupId: groupID, - Member: &group.Member{ - Address: member1, - Weight: "2", - AddedAt: s.blockTime, - }, }, }, }, "update member with same data": { req: &group.MsgUpdateGroupMembers{ - GroupId: groupID, - Admin: myAdmin, - MemberUpdates: []group.MemberRequest{{ - Address: member1, - Weight: "1", - }}, + GroupId: groupID, + Admin: myAdmin, + MemberUpdates: []group.MemberRequest{{Address: member1, Weight: "1"}}, }, expGroup: &group.GroupInfo{ Id: groupID, Admin: myAdmin, - TotalWeight: "1", + TotalWeight: "3", Version: 2, CreatedAt: s.blockTime, }, expMembers: []*group.GroupMember{ { GroupId: groupID, - Member: &group.Member{ - Address: member1, - Weight: "1", - AddedAt: s.blockTime, - }, + Member: &group.Member{Address: member1, Weight: "1", AddedAt: s.blockTime}, + }, + { + GroupId: groupID, + Member: &group.Member{Address: member2, Weight: "2", AddedAt: s.blockTime}, }, }, }, @@ -406,58 +386,51 @@ func (s *TestSuite) TestUpdateGroupMembers() { GroupId: groupID, Admin: myAdmin, MemberUpdates: []group.MemberRequest{ - { - Address: member1, - Weight: "0", - }, - { - Address: member2, - Weight: "1", - }, + {Address: member1, Weight: "0"}, + {Address: member3, Weight: "1"}, }, }, expGroup: &group.GroupInfo{ Id: groupID, Admin: myAdmin, - TotalWeight: "1", + TotalWeight: "3", Version: 2, CreatedAt: s.blockTime, }, - expMembers: []*group.GroupMember{{ - GroupId: groupID, - Member: &group.Member{ - Address: member2, - Weight: "1", - AddedAt: s.sdkCtx.BlockTime(), + expMembers: []*group.GroupMember{ + { + Member: &group.Member{Address: member3, Weight: "1", AddedAt: s.blockTime}, + GroupId: groupID, }, - }}, + { + Member: &group.Member{Address: member2, Weight: "2", AddedAt: s.blockTime}, + GroupId: groupID, + }}, }, "remove existing member": { req: &group.MsgUpdateGroupMembers{ - GroupId: groupID, - Admin: myAdmin, - MemberUpdates: []group.MemberRequest{{ - Address: member1, - Weight: "0", - }}, + GroupId: groupID, + Admin: myAdmin, + MemberUpdates: []group.MemberRequest{{Address: member1, Weight: "0"}}, }, expGroup: &group.GroupInfo{ Id: groupID, Admin: myAdmin, - TotalWeight: "0", + TotalWeight: "2", Version: 2, CreatedAt: s.blockTime, }, - expMembers: []*group.GroupMember{}, + expMembers: []*group.GroupMember{ + { + Member: &group.Member{Address: member2, Weight: "2", AddedAt: s.blockTime}, + GroupId: groupID, + }}, }, "remove unknown member": { req: &group.MsgUpdateGroupMembers{ - GroupId: groupID, - Admin: myAdmin, - MemberUpdates: []group.MemberRequest{{ - Address: addr4.String(), - Weight: "0", - }}, + GroupId: groupID, + Admin: myAdmin, + MemberUpdates: []group.MemberRequest{{Address: unknownAddr, Weight: "0"}}, }, expErr: true, expGroup: &group.GroupInfo{ @@ -467,66 +440,73 @@ func (s *TestSuite) TestUpdateGroupMembers() { Version: 1, CreatedAt: s.blockTime, }, - expMembers: []*group.GroupMember{{ - GroupId: groupID, - Member: &group.Member{ - Address: member1, - Weight: "1", - }, - }}, + expMembers: []*group.GroupMember{ + { + GroupId: groupID, + Member: &group.Member{Address: member1, Weight: "1"}, + }, { + Member: &group.Member{Address: member2, Weight: "2", AddedAt: s.blockTime}, + GroupId: groupID, + }}, }, "with wrong admin": { req: &group.MsgUpdateGroupMembers{ - GroupId: groupID, - Admin: addr3.String(), - MemberUpdates: []group.MemberRequest{{ - Address: member1, - Weight: "2", - }}, + GroupId: groupID, + Admin: unknownAddr, + MemberUpdates: []group.MemberRequest{{Address: member1, Weight: "2"}}, }, expErr: true, expErrMsg: "not group admin", expGroup: &group.GroupInfo{ Id: groupID, Admin: myAdmin, - TotalWeight: "1", + TotalWeight: "3", Version: 1, CreatedAt: s.blockTime, }, expMembers: []*group.GroupMember{{ GroupId: groupID, - Member: &group.Member{ - Address: member1, - Weight: "1", - }, + Member: &group.Member{Address: member1, Weight: "1"}, + }, { + Member: &group.Member{Address: member2, Weight: "2", AddedAt: s.blockTime}, + GroupId: groupID, }}, }, "with unknown groupID": { req: &group.MsgUpdateGroupMembers{ - GroupId: 999, - Admin: myAdmin, - MemberUpdates: []group.MemberRequest{{ - Address: member1, - Weight: "2", - }}, + GroupId: 999, + Admin: myAdmin, + MemberUpdates: []group.MemberRequest{{Address: member1, Weight: "2"}}, }, expErr: true, expErrMsg: "not found", expGroup: &group.GroupInfo{ Id: groupID, Admin: myAdmin, - TotalWeight: "1", + TotalWeight: "3", Version: 1, CreatedAt: s.blockTime, }, expMembers: []*group.GroupMember{{ GroupId: groupID, - Member: &group.Member{ - Address: member1, - Weight: "1", - }, + Member: &group.Member{Address: member1, Weight: "1"}, + }, { + Member: &group.Member{Address: member2, Weight: "2", AddedAt: s.blockTime}, + GroupId: groupID, }}, }, + "remove all members": { + req: &group.MsgUpdateGroupMembers{ + GroupId: groupID, + Admin: myAdmin, + MemberUpdates: []group.MemberRequest{ + {Address: member1, Weight: "0"}, + {Address: member2, Weight: "0"}, + }, + }, + expErr: true, + expErrMsg: "group must not be empty", + }, } for msg, spec := range specs { spec := spec diff --git a/x/group/simulation/operations.go b/x/group/simulation/operations.go index 94576d36d615..e7ec1911e56b 100644 --- a/x/group/simulation/operations.go +++ b/x/group/simulation/operations.go @@ -629,7 +629,7 @@ func SimulateMsgUpdateGroupMembers( // set existing random group member weight to zero to remove from the group existigMembers := res.Members - if len(existigMembers) > 0 { + if len(existigMembers) > 1 { memberToRemove := existigMembers[r.Intn(len(existigMembers))] var isDuplicateMember bool for idx, m := range members { diff --git a/x/group/types.go b/x/group/types.go index 529b5be66d24..4c498b1b1b7b 100644 --- a/x/group/types.go +++ b/x/group/types.go @@ -209,7 +209,9 @@ func (p PercentageDecisionPolicy) Allow(tally TallyResult, totalPower string) (D if err != nil { return DecisionPolicyResult{}, errorsmod.Wrap(err, "total power") } - + if totalPowerDec.IsZero() { + return DecisionPolicyResult{Allow: false, Final: true}, nil + } yesPercentage, err := yesCount.Quo(totalPowerDec) if err != nil { return DecisionPolicyResult{}, err diff --git a/x/group/types_test.go b/x/group/types_test.go index afad812a39d5..c69bb2f5ce0a 100644 --- a/x/group/types_test.go +++ b/x/group/types_test.go @@ -239,6 +239,28 @@ func TestPercentageDecisionPolicyAllow(t *testing.T) { }, false, }, + { + "empty total power", + &group.PercentageDecisionPolicy{ + Percentage: "0.5", + Windows: &group.DecisionPolicyWindows{ + VotingPeriod: time.Second * 100, + }, + }, + &group.TallyResult{ + YesCount: "1", + NoCount: "0", + AbstainCount: "0", + NoWithVetoCount: "0", + }, + "0", + time.Second * 50, + group.DecisionPolicyResult{ + Allow: false, + Final: true, + }, + false, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) {