diff --git a/modules/apps/27-interchain-accounts/controller/types/msgs.go b/modules/apps/27-interchain-accounts/controller/types/msgs.go index 9f9f0434195..7ecf66228a9 100644 --- a/modules/apps/27-interchain-accounts/controller/types/msgs.go +++ b/modules/apps/27-interchain-accounts/controller/types/msgs.go @@ -10,6 +10,8 @@ import ( host "github.com/cosmos/ibc-go/v6/modules/core/24-host" ) +const MaximumOwnerLength = 2048 // maximum length of the owner in bytes (value chosen arbitrarily) + var _ sdk.Msg = &MsgRegisterInterchainAccount{} // NewMsgRegisterInterchainAccount creates a new instance of MsgRegisterInterchainAccount @@ -31,6 +33,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 } @@ -64,6 +70,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") } diff --git a/modules/apps/27-interchain-accounts/controller/types/msgs_test.go b/modules/apps/27-interchain-accounts/controller/types/msgs_test.go index 3f8ccb9d881..de04b949b45 100644 --- a/modules/apps/27-interchain-accounts/controller/types/msgs_test.go +++ b/modules/apps/27-interchain-accounts/controller/types/msgs_test.go @@ -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 { @@ -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() { diff --git a/modules/apps/27-interchain-accounts/types/account_test.go b/modules/apps/27-interchain-accounts/types/account_test.go index d606c39bc0d..1bf01e7015a 100644 --- a/modules/apps/27-interchain-accounts/types/account_test.go +++ b/modules/apps/27-interchain-accounts/types/account_test.go @@ -78,7 +78,7 @@ func (suite *TypesTestSuite) TestValidateAccountAddress() { }, { "address is too long", - ibctesting.LongString, + ibctesting.GenerateString(uint(types.DefaultMaxAddrLength) + 1), false, }, } diff --git a/modules/apps/29-fee/types/msgs.go b/modules/apps/29-fee/types/msgs.go index 5e2f791748f..14ced6cdc69 100644 --- a/modules/apps/29-fee/types/msgs.go +++ b/modules/apps/29-fee/types/msgs.go @@ -14,6 +14,7 @@ import ( const ( TypeMsgPayPacketFee = "payPacketFee" TypeMsgPayPacketFeeAsync = "payPacketFeeAsync" + MaximumCounterpartyPayeeLength = 2048 // maximum length of the counterparty payee in bytes (value chosen arbitrarily) ) // NewMsgRegisterPayee creates a new instance of MsgRegisterPayee @@ -92,6 +93,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 } diff --git a/modules/apps/29-fee/types/msgs_test.go b/modules/apps/29-fee/types/msgs_test.go index e34e0738318..2059c814b27 100644 --- a/modules/apps/29-fee/types/msgs_test.go +++ b/modules/apps/29-fee/types/msgs_test.go @@ -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 { diff --git a/modules/apps/transfer/types/errors.go b/modules/apps/transfer/types/errors.go index d4f85cf4fa7..c2e1d2141b1 100644 --- a/modules/apps/transfer/types/errors.go +++ b/modules/apps/transfer/types/errors.go @@ -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") ) diff --git a/modules/apps/transfer/types/msgs.go b/modules/apps/transfer/types/msgs.go index 65b2202a9db..b8bd88d07bd 100644 --- a/modules/apps/transfer/types/msgs.go +++ b/modules/apps/transfer/types/msgs.go @@ -12,7 +12,9 @@ import ( // msg types const ( - TypeMsgTransfer = "transfer" + TypeMsgTransfer = "transfer" + 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) ) // NewMsgTransfer creates a new MsgTransfer instance @@ -71,6 +73,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) } diff --git a/modules/apps/transfer/types/msgs_test.go b/modules/apps/transfer/types/msgs_test.go index 4268a282ef4..a8edd4045a8 100644 --- a/modules/apps/transfer/types/msgs_test.go +++ b/modules/apps/transfer/types/msgs_test.go @@ -1,4 +1,4 @@ -package types +package types_test import ( "fmt" @@ -8,7 +8,9 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/stretchr/testify/require" + "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types" clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types" + ibctesting "github.com/cosmos/ibc-go/v6/testing" ) // define constants used for testing @@ -41,20 +43,20 @@ var ( // TestMsgTransferRoute tests Route for MsgTransfer func TestMsgTransferRoute(t *testing.T) { - msg := NewMsgTransfer(validPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, "") + msg := types.NewMsgTransfer(validPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, "") - require.Equal(t, RouterKey, msg.Route()) + require.Equal(t, types.RouterKey, msg.Route()) } // TestMsgTransferType tests Type for MsgTransfer func TestMsgTransferType(t *testing.T) { - msg := NewMsgTransfer(validPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, "") + msg := types.NewMsgTransfer(validPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, "") require.Equal(t, "transfer", msg.Type()) } func TestMsgTransferGetSignBytes(t *testing.T) { - msg := NewMsgTransfer(validPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, "") + msg := types.NewMsgTransfer(validPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, "") expected := fmt.Sprintf(`{"type":"cosmos-sdk/MsgTransfer","value":{"receiver":"%s","sender":"%s","source_channel":"testchannel","source_port":"testportid","timeout_height":{"revision_height":"10"},"token":{"amount":"100","denom":"atom"}}}`, addr2, addr1) require.NotPanics(t, func() { res := msg.GetSignBytes() @@ -66,23 +68,25 @@ func TestMsgTransferGetSignBytes(t *testing.T) { func TestMsgTransferValidation(t *testing.T) { testCases := []struct { name string - msg *MsgTransfer + msg *types.MsgTransfer expPass bool }{ - {"valid msg with base denom", NewMsgTransfer(validPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, ""), true}, - {"valid msg with trace hash", NewMsgTransfer(validPort, validChannel, ibcCoin, addr1, addr2, timeoutHeight, 0, ""), true}, - {"invalid ibc denom", NewMsgTransfer(validPort, validChannel, invalidIBCCoin, addr1, addr2, timeoutHeight, 0, ""), false}, - {"too short port id", NewMsgTransfer(invalidShortPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false}, - {"too long port id", NewMsgTransfer(invalidLongPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false}, - {"port id contains non-alpha", NewMsgTransfer(invalidPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false}, - {"too short channel id", NewMsgTransfer(validPort, invalidShortChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false}, - {"too long channel id", NewMsgTransfer(validPort, invalidLongChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false}, - {"channel id contains non-alpha", NewMsgTransfer(validPort, invalidChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false}, - {"invalid denom", NewMsgTransfer(validPort, validChannel, invalidDenomCoin, addr1, addr2, timeoutHeight, 0, ""), false}, - {"zero coin", NewMsgTransfer(validPort, validChannel, zeroCoin, addr1, addr2, timeoutHeight, 0, ""), false}, - {"missing sender address", NewMsgTransfer(validPort, validChannel, coin, emptyAddr, addr2, timeoutHeight, 0, ""), false}, - {"missing recipient address", NewMsgTransfer(validPort, validChannel, coin, addr1, "", timeoutHeight, 0, ""), false}, - {"empty coin", NewMsgTransfer(validPort, validChannel, sdk.Coin{}, addr1, addr2, timeoutHeight, 0, ""), false}, + {"valid msg with base denom", types.NewMsgTransfer(validPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, ""), true}, + {"valid msg with trace hash", types.NewMsgTransfer(validPort, validChannel, ibcCoin, addr1, addr2, timeoutHeight, 0, ""), true}, + {"invalid ibc denom", types.NewMsgTransfer(validPort, validChannel, invalidIBCCoin, addr1, addr2, timeoutHeight, 0, ""), false}, + {"too short port id", types.NewMsgTransfer(invalidShortPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false}, + {"too long port id", types.NewMsgTransfer(invalidLongPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false}, + {"port id contains non-alpha", types.NewMsgTransfer(invalidPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false}, + {"too short channel id", types.NewMsgTransfer(validPort, invalidShortChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false}, + {"too long channel id", types.NewMsgTransfer(validPort, invalidLongChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false}, + {"too long memo", types.NewMsgTransfer(validPort, validChannel, coin, addr1, addr2, timeoutHeight, 0, ibctesting.GenerateString(types.MaximumMemoLength+1)), false}, + {"channel id contains non-alpha", types.NewMsgTransfer(validPort, invalidChannel, coin, addr1, addr2, timeoutHeight, 0, ""), false}, + {"invalid denom", types.NewMsgTransfer(validPort, validChannel, invalidDenomCoin, addr1, addr2, timeoutHeight, 0, ""), false}, + {"zero coin", types.NewMsgTransfer(validPort, validChannel, zeroCoin, addr1, addr2, timeoutHeight, 0, ""), false}, + {"missing sender address", types.NewMsgTransfer(validPort, validChannel, coin, emptyAddr, addr2, timeoutHeight, 0, ""), false}, + {"missing recipient address", types.NewMsgTransfer(validPort, validChannel, coin, addr1, "", timeoutHeight, 0, ""), false}, + {"too long recipient address", types.NewMsgTransfer(validPort, validChannel, coin, addr1, ibctesting.GenerateString(types.MaximumReceiverLength+1), timeoutHeight, 0, ""), false}, + {"empty coin", types.NewMsgTransfer(validPort, validChannel, sdk.Coin{}, addr1, addr2, timeoutHeight, 0, ""), false}, } for i, tc := range testCases { @@ -99,7 +103,7 @@ func TestMsgTransferValidation(t *testing.T) { func TestMsgTransferGetSigners(t *testing.T) { addr := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) - msg := NewMsgTransfer(validPort, validChannel, coin, addr.String(), addr2, timeoutHeight, 0, "") + msg := types.NewMsgTransfer(validPort, validChannel, coin, addr.String(), addr2, timeoutHeight, 0, "") res := msg.GetSigners() require.Equal(t, []sdk.AccAddress{addr}, res) diff --git a/modules/apps/transfer/types/packet_test.go b/modules/apps/transfer/types/packet_test.go index cef36ab2211..cd267bc81ff 100644 --- a/modules/apps/transfer/types/packet_test.go +++ b/modules/apps/transfer/types/packet_test.go @@ -1,9 +1,11 @@ -package types +package types_test import ( "testing" "github.com/stretchr/testify/require" + + "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types" ) const ( @@ -17,19 +19,19 @@ const ( func TestFungibleTokenPacketDataValidateBasic(t *testing.T) { testCases := []struct { name string - packetData FungibleTokenPacketData + packetData types.FungibleTokenPacketData expPass bool }{ - {"valid packet", NewFungibleTokenPacketData(denom, amount, addr1, addr2, ""), true}, - {"valid packet with memo", NewFungibleTokenPacketData(denom, amount, addr1, addr2, "memo"), true}, - {"valid packet with large amount", NewFungibleTokenPacketData(denom, largeAmount, addr1, addr2, ""), true}, - {"invalid denom", NewFungibleTokenPacketData("", amount, addr1, addr2, ""), false}, - {"invalid empty amount", NewFungibleTokenPacketData(denom, "", addr1, addr2, ""), false}, - {"invalid zero amount", NewFungibleTokenPacketData(denom, "0", addr1, addr2, ""), false}, - {"invalid negative amount", NewFungibleTokenPacketData(denom, "-1", addr1, addr2, ""), false}, - {"invalid large amount", NewFungibleTokenPacketData(denom, invalidLargeAmount, addr1, addr2, ""), false}, - {"missing sender address", NewFungibleTokenPacketData(denom, amount, emptyAddr, addr2, ""), false}, - {"missing recipient address", NewFungibleTokenPacketData(denom, amount, addr1, emptyAddr, ""), false}, + {"valid packet", types.NewFungibleTokenPacketData(denom, amount, addr1, addr2, ""), true}, + {"valid packet with memo", types.NewFungibleTokenPacketData(denom, amount, addr1, addr2, "memo"), true}, + {"valid packet with large amount", types.NewFungibleTokenPacketData(denom, largeAmount, addr1, addr2, ""), true}, + {"invalid denom", types.NewFungibleTokenPacketData("", amount, addr1, addr2, ""), false}, + {"invalid empty amount", types.NewFungibleTokenPacketData(denom, "", addr1, addr2, ""), false}, + {"invalid zero amount", types.NewFungibleTokenPacketData(denom, "0", addr1, addr2, ""), false}, + {"invalid negative amount", types.NewFungibleTokenPacketData(denom, "-1", addr1, addr2, ""), false}, + {"invalid large amount", types.NewFungibleTokenPacketData(denom, invalidLargeAmount, addr1, addr2, ""), false}, + {"missing sender address", types.NewFungibleTokenPacketData(denom, amount, emptyAddr, addr2, ""), false}, + {"missing recipient address", types.NewFungibleTokenPacketData(denom, amount, addr1, emptyAddr, ""), false}, } for i, tc := range testCases { diff --git a/testing/utils.go b/testing/utils.go index f9f64bf72bd..67bf6c94631 100644 --- a/testing/utils.go +++ b/testing/utils.go @@ -1,6 +1,7 @@ package ibctesting import ( + "math/rand" "testing" "github.com/stretchr/testify/require" @@ -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) +} diff --git a/testing/values.go b/testing/values.go index 7cff55583bc..1736bd3f4e2 100644 --- a/testing/values.go +++ b/testing/values.go @@ -40,7 +40,7 @@ const ( Title = "title" Description = "description" - LongString = "LoremipsumdolorsitameconsecteturadipiscingeliseddoeiusmodtemporincididuntutlaboreetdoloremagnaaliquUtenimadminimveniamquisnostrudexercitationullamcolaborisnisiutaliquipexeacommodoconsequDuisauteiruredolorinreprehenderitinvoluptateelitsseillumoloreufugiatnullaariaturEcepteurintoccaectupidatatonroidentuntnulpauifficiaeseruntmollitanimidestlaborum" + charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789" ) var (