Skip to content

Commit

Permalink
Merge pull request from GHSA-4j93-fm92-rp4m
Browse files Browse the repository at this point in the history
* fix(x/auth/vesting): Add `BlockedAddr` check in `CreatePeriodicVestingAccount`

* updates
  • Loading branch information
julienrbrt authored Feb 19, 2024
1 parent 18ea4c5 commit 7dbed2f
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Bug Fixes

* (x/auth/vesting) [GHSA-4j93-fm92-rp4m](#bug-fixes) Add `BlockedAddr` check in `CreatePeriodicVestingAccount`.
* (baseapp) [#19338](https://github.com/cosmos/cosmos-sdk/pull/19338) Set HeaderInfo in context when calling `setState`.
* (baseapp): [#19200](https://github.com/cosmos/cosmos-sdk/pull/19200) Ensure that sdk side ve math matches cometbft.
* [#19106](https://github.com/cosmos/cosmos-sdk/pull/19106) Allow empty public keys when setting signatures. Public keys aren't needed for every transaction.
Expand Down
5 changes: 3 additions & 2 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@ Notably, we added and fixed the following:

* Adds in-place testnet CLI command for creating testnets from local state (kudos to @czarcas7ic)
* Multiple fixes in baseapp, with fixes in `DefaultProposalHandler` and vote extensions
* <>
* Add a missed check in `x/auth/vesting`: [GHSA-4j93-fm92-rp4m](https://github.com/cosmos/cosmos-sdk/security/advisories/GHSA-4j93-fm92-rp4m)

We recommended to upgrade to this patch release as soon as possible.
We recommended to upgrade to this patch release as soon as possible.
When upgrading from <= v0.50.3, please ensure that 2/3 of the validator power upgrade to v0.50.4.

## 📝 Changelog

Expand Down
4 changes: 4 additions & 0 deletions x/auth/vesting/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ func (s msgServer) CreatePeriodicVestingAccount(goCtx context.Context, msg *type
totalCoins = totalCoins.Add(period.Amount...)
}

if s.BankKeeper.BlockedAddr(to) {
return nil, errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to receive funds", msg.ToAddress)
}

ctx := sdk.UnwrapSDKContext(goCtx)
if acc := s.AccountKeeper.GetAccount(ctx, to); acc != nil {
return nil, errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "account %s already exists", msg.ToAddress)
Expand Down
56 changes: 56 additions & 0 deletions x/auth/vesting/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,21 @@ func (s *VestingTestSuite) TestCreateVestingAccount() {
expErr: true,
expErrMsg: "already exists",
},
"create for blocked account": {
preRun: func() {
s.bankKeeper.EXPECT().IsSendEnabledCoins(gomock.Any(), fooCoin).Return(nil)
s.bankKeeper.EXPECT().BlockedAddr(to1Addr).Return(true)
},
input: vestingtypes.NewMsgCreateVestingAccount(
fromAddr,
to1Addr,
sdk.Coins{fooCoin},
time.Now().Unix(),
true,
),
expErr: true,
expErrMsg: "not allowed to receive funds",
},
"create a valid delayed vesting account": {
preRun: func() {
s.bankKeeper.EXPECT().IsSendEnabledCoins(gomock.Any(), fooCoin).Return(nil)
Expand Down Expand Up @@ -235,6 +250,22 @@ func (s *VestingTestSuite) TestCreatePermanentLockedAccount() {
expErr: true,
expErrMsg: "already exists",
},
"create for blocked account": {
preRun: func() {
toAcc := s.accountKeeper.NewAccountWithAddress(s.ctx, to1Addr)
s.bankKeeper.EXPECT().IsSendEnabledCoins(gomock.Any(), fooCoin).Return(nil)
s.bankKeeper.EXPECT().BlockedAddr(to1Addr).Return(true)
s.accountKeeper.SetAccount(s.ctx, toAcc)
},
input: vestingtypes.NewMsgCreatePermanentLockedAccount(
fromAddr,
to1Addr,
sdk.Coins{fooCoin},
),
expErr: true,
expErrMsg: "not allowed to receive funds",
},

"create a valid permanent locked account": {
preRun: func() {
s.bankKeeper.EXPECT().IsSendEnabledCoins(gomock.Any(), fooCoin).Return(nil)
Expand Down Expand Up @@ -359,6 +390,7 @@ func (s *VestingTestSuite) TestCreatePeriodicVestingAccount() {
{
name: "create for existing account",
preRun: func() {
s.bankKeeper.EXPECT().BlockedAddr(to1Addr).Return(false)
toAcc := s.accountKeeper.NewAccountWithAddress(s.ctx, to1Addr)
s.accountKeeper.SetAccount(s.ctx, toAcc)
},
Expand All @@ -376,10 +408,34 @@ func (s *VestingTestSuite) TestCreatePeriodicVestingAccount() {
expErr: true,
expErrMsg: "already exists",
},
{
name: "create for blocked address",
preRun: func() {
s.bankKeeper.EXPECT().BlockedAddr(to2Addr).Return(true)
},
input: vestingtypes.NewMsgCreatePeriodicVestingAccount(
fromAddr,
to2Addr,
time.Now().Unix(),
[]vestingtypes.Period{
{
Length: 10,
Amount: sdk.NewCoins(periodCoin),
},
{
Length: 20,
Amount: sdk.NewCoins(fooCoin),
},
},
),
expErr: true,
expErrMsg: "not allowed to receive funds",
},
{
name: "create a valid periodic vesting account",
preRun: func() {
s.bankKeeper.EXPECT().IsSendEnabledCoins(gomock.Any(), periodCoin.Add(fooCoin)).Return(nil)
s.bankKeeper.EXPECT().BlockedAddr(to2Addr).Return(false)
s.bankKeeper.EXPECT().SendCoins(gomock.Any(), fromAddr, to2Addr, gomock.Any()).Return(nil)
},
input: vestingtypes.NewMsgCreatePeriodicVestingAccount(
Expand Down

0 comments on commit 7dbed2f

Please sign in to comment.