Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

imp(statemachine)!: check length of unbounded string fields in messages #4992

Merged
merged 13 commits into from
Oct 31, 2023
10 changes: 10 additions & 0 deletions modules/apps/27-interchain-accounts/controller/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
)

const MaximumOwnerLength = 2048 // maximum length of the owner in bytes (value chosen arbitrarily)

var (
_ sdk.Msg = (*MsgRegisterInterchainAccount)(nil)
_ sdk.Msg = (*MsgSendTx)(nil)
Expand Down Expand Up @@ -41,6 +43,10 @@ func (msg MsgRegisterInterchainAccount) ValidateBasic() error {
return errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "owner address cannot be empty")
}

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

return nil
}

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

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

if err := msg.PacketData.ValidateBasic(); err != nil {
return errorsmod.Wrap(err, "invalid interchain account packet data")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,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 @@ -121,6 +128,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
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,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 @@ -12,6 +12,8 @@ import (
ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
)

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 @@ -100,6 +102,10 @@ func (msg MsgRegisterCounterpartyPayee) ValidateBasic() error {
return ErrCounterpartyPayeeEmpty
}

if len(msg.CounterpartyPayee) > MaximumCounterpartyPayeeLength {
return errorsmod.Wrapf(ibcerrors.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 @@ -139,6 +139,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 = errorsmod.Register(ModuleName, 8, "fungible token transfers to this chain are disabled")
ErrMaxTransferChannels = errorsmod.Register(ModuleName, 9, "max transfer channels")
ErrInvalidAuthorization = errorsmod.Register(ModuleName, 10, "invalid transfer authorization")
ErrInvalidMemo = errorsmod.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 @@ -12,6 +12,11 @@ import (
ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
)

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 = (*MsgUpdateParams)(nil)
_ sdk.Msg = (*MsgTransfer)(nil)
Expand Down Expand Up @@ -91,6 +96,12 @@ func (msg MsgTransfer) ValidateBasic() error {
if strings.TrimSpace(msg.Receiver) == "" {
return errorsmod.Wrap(ibcerrors.ErrInvalidAddress, "missing recipient address")
}
if len(msg.Receiver) > MaximumReceiverLength {
return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "recipient address must not exceed %d bytes", MaximumReceiverLength)
}
if len(msg.Memo) > MaximumMemoLength {
return errorsmod.Wrapf(ErrInvalidMemo, "memo must not exceed %d bytes", MaximumMemoLength)
}
return ValidateIBCDenom(msg.Token.Denom)
}

Expand Down
2 changes: 2 additions & 0 deletions modules/apps/transfer/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,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"

"github.com/stretchr/testify/require"
Expand All @@ -23,3 +24,12 @@ func ApplyValSetChanges(tb testing.TB, 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 @@ -43,7 +43,8 @@ const (
Title = "title"
Description = "description"

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

var (
Expand Down
Loading