Skip to content

Commit

Permalink
chore: backport #4992 to release/v7.6.x (#6625)
Browse files Browse the repository at this point in the history
* chore: backport #4992 to release/v7.6.x

* changelog
  • Loading branch information
crodriguezvega authored Jun 19, 2024
1 parent 9670945 commit a0dec5a
Show file tree
Hide file tree
Showing 11 changed files with 71 additions and 5 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ Ref: https://keepachangelog.com/en/1.0.0/

### State Machine Breaking

### Improvements
* (apps/transfer, apps/27-interchain-accounts, app/29-fee) #4992 Set validation for length of string fields.

* (apps/27-interchain-accounts) [\#6436](https://github.com/cosmos/ibc-go/pull/6436) Refactor ICA host keeper instantiation method to avoid panic related to proto files.
### Improvements

### Features

Expand Down
15 changes: 14 additions & 1 deletion modules/apps/27-interchain-accounts/controller/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ import (
host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
)

var _ sdk.Msg = &MsgRegisterInterchainAccount{}
const MaximumOwnerLength = 2048 // maximum length of the owner in bytes (value chosen arbitrarily)

var (
_ sdk.Msg = (*MsgRegisterInterchainAccount)(nil)
_ sdk.Msg = (*MsgSendTx)(nil)
)

// NewMsgRegisterInterchainAccountWithOrdering creates a new instance of MsgRegisterInterchainAccount.
func NewMsgRegisterInterchainAccountWithOrdering(connectionID, owner, version string, ordering channeltypes.Order) *MsgRegisterInterchainAccount {
Expand Down Expand Up @@ -45,6 +50,10 @@ func (msg MsgRegisterInterchainAccount) ValidateBasic() error {
return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "owner address cannot be empty")
}

if len(msg.Owner) > MaximumOwnerLength {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "owner address must not exceed %d bytes", MaximumOwnerLength)
}

return nil
}

Expand Down Expand Up @@ -78,6 +87,10 @@ func (msg MsgSendTx) ValidateBasic() error {
return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "owner address cannot be empty")
}

if len(msg.Owner) > MaximumOwnerLength {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "owner address must not exceed %d bytes", MaximumOwnerLength)
}

if err := msg.PacketData.ValidateBasic(); err != nil {
return sdkerrors.Wrap(err, "invalid interchain account packet data")
}
Expand Down
14 changes: 14 additions & 0 deletions modules/apps/27-interchain-accounts/controller/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ func TestMsgRegisterInterchainAccountValidateBasic(t *testing.T) {
},
false,
},
{
"owner address is too long",
func() {
msg.Owner = ibctesting.GenerateString(types.MaximumOwnerLength + 1)
},
false,
},
}

for i, tc := range testCases {
Expand Down Expand Up @@ -118,6 +125,13 @@ func TestMsgSendTxValidateBasic(t *testing.T) {
},
false,
},
{
"owner address is too long",
func() {
msg.Owner = ibctesting.GenerateString(types.MaximumOwnerLength + 1)
},
false,
},
{
"relative timeout is not set",
func() {
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/27-interchain-accounts/types/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (suite *TypesTestSuite) TestValidateAccountAddress() {
},
{
"address is too long",
ibctesting.LongString,
ibctesting.GenerateString(uint(types.DefaultMaxAddrLength) + 1),
false,
},
}
Expand Down
6 changes: 6 additions & 0 deletions modules/apps/29-fee/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ const (
TypeMsgPayPacketFeeAsync = "payPacketFeeAsync"
)

const MaximumCounterpartyPayeeLength = 2048 // maximum length of the counterparty payee in bytes (value chosen arbitrarily)

var (
_ sdk.Msg = (*MsgRegisterPayee)(nil)
_ sdk.Msg = (*MsgRegisterCounterpartyPayee)(nil)
Expand Down Expand Up @@ -102,6 +104,10 @@ func (msg MsgRegisterCounterpartyPayee) ValidateBasic() error {
return ErrCounterpartyPayeeEmpty
}

if len(msg.CounterpartyPayee) > MaximumCounterpartyPayeeLength {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "counterparty payee address must not exceed %d bytes", MaximumCounterpartyPayeeLength)
}

return nil
}

Expand Down
7 changes: 7 additions & 0 deletions modules/apps/29-fee/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,13 @@ func TestMsgRegisterCountepartyPayeeValidation(t *testing.T) {
},
false,
},
{
"invalid counterparty payee address: too long",
func() {
msg.CounterpartyPayee = ibctesting.GenerateString(types.MaximumCounterpartyPayeeLength + 1)
},
false,
},
}

for i, tc := range testCases {
Expand Down
1 change: 1 addition & 0 deletions modules/apps/transfer/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,5 @@ var (
ErrReceiveDisabled = sdkerrors.Register(ModuleName, 8, "fungible token transfers to this chain are disabled")
ErrMaxTransferChannels = sdkerrors.Register(ModuleName, 9, "max transfer channels")
ErrInvalidAuthorization = sdkerrors.Register(ModuleName, 10, "invalid transfer authorization")
ErrInvalidMemo = sdkerrors.Register(ModuleName, 11, "invalid memo")
)
11 changes: 11 additions & 0 deletions modules/apps/transfer/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ const (
TypeMsgTransfer = "transfer"
)

const (
MaximumReceiverLength = 2048 // maximum length of the receiver address in bytes (value chosen arbitrarily)
MaximumMemoLength = 32768 // maximum length of the memo in bytes (value chosen arbitrarily)
)

var (
_ sdk.Msg = (*MsgTransfer)(nil)
_ legacytx.LegacyMsg = (*MsgTransfer)(nil)
Expand Down Expand Up @@ -77,6 +82,12 @@ func (msg MsgTransfer) ValidateBasic() error {
if strings.TrimSpace(msg.Receiver) == "" {
return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "missing recipient address")
}
if len(msg.Receiver) > MaximumReceiverLength {
return sdkerrors.Wrapf(sdkerrors.ErrInvalidAddress, "recipient address must not exceed %d bytes", MaximumReceiverLength)
}
if len(msg.Memo) > MaximumMemoLength {
return sdkerrors.Wrapf(ErrInvalidMemo, "memo must not exceed %d bytes", MaximumMemoLength)
}
return ValidateIBCDenom(msg.Token.Denom)
}

Expand Down
3 changes: 3 additions & 0 deletions modules/apps/transfer/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/cosmos/ibc-go/v7/modules/apps/transfer/types"
clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
ibctesting "github.com/cosmos/ibc-go/v7/testing"
)

// define constants used for testing
Expand Down Expand Up @@ -71,11 +72,13 @@ func TestMsgTransferValidation(t *testing.T) {
{"port id contains non-alpha", types.NewMsgTransfer(invalidPort, validChannel, coin, sender, receiver, timeoutHeight, 0, ""), false},
{"too short channel id", types.NewMsgTransfer(validPort, invalidShortChannel, coin, sender, receiver, timeoutHeight, 0, ""), false},
{"too long channel id", types.NewMsgTransfer(validPort, invalidLongChannel, coin, sender, receiver, timeoutHeight, 0, ""), false},
{"too long memo", types.NewMsgTransfer(validPort, validChannel, coin, sender, receiver, timeoutHeight, 0, ibctesting.GenerateString(types.MaximumMemoLength+1)), false},
{"channel id contains non-alpha", types.NewMsgTransfer(validPort, invalidChannel, coin, sender, receiver, timeoutHeight, 0, ""), false},
{"invalid denom", types.NewMsgTransfer(validPort, validChannel, invalidDenomCoin, sender, receiver, timeoutHeight, 0, ""), false},
{"zero coin", types.NewMsgTransfer(validPort, validChannel, zeroCoin, sender, receiver, timeoutHeight, 0, ""), false},
{"missing sender address", types.NewMsgTransfer(validPort, validChannel, coin, emptyAddr, receiver, timeoutHeight, 0, ""), false},
{"missing recipient address", types.NewMsgTransfer(validPort, validChannel, coin, sender, "", timeoutHeight, 0, ""), false},
{"too long recipient address", types.NewMsgTransfer(validPort, validChannel, coin, sender, ibctesting.GenerateString(types.MaximumReceiverLength+1), timeoutHeight, 0, ""), false},
{"empty coin", types.NewMsgTransfer(validPort, validChannel, sdk.Coin{}, sender, receiver, timeoutHeight, 0, ""), false},
}

Expand Down
10 changes: 10 additions & 0 deletions testing/utils.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package ibctesting

import (
"math/rand"
"testing"

abci "github.com/cometbft/cometbft/abci/types"
Expand All @@ -21,3 +22,12 @@ func ApplyValSetChanges(t *testing.T, valSet *tmtypes.ValidatorSet, valUpdates [

return newVals
}

// GenerateString generates a random string of the given length in bytes
func GenerateString(length uint) string {
bytes := make([]byte, length)
for i := range bytes {
bytes[i] = charset[rand.Intn(len(charset))]
}
return string(bytes)
}
3 changes: 2 additions & 1 deletion testing/values.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ const (
Title = "title"
Description = "description"

LongString = "LoremipsumdolorsitameconsecteturadipiscingeliseddoeiusmodtemporincididuntutlaboreetdoloremagnaaliquUtenimadminimveniamquisnostrudexercitationullamcolaborisnisiutaliquipexeacommodoconsequDuisauteiruredolorinreprehenderitinvoluptateelitsseillumoloreufugiatnullaariaturEcepteurintoccaectupidatatonroidentuntnulpauifficiaeseruntmollitanimidestlaborum"
// character set used for generating a random string in GenerateString
charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
)

var (
Expand Down

0 comments on commit a0dec5a

Please sign in to comment.