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
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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

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 addresss must not exceed %d bytes", MaximumOwnerLength)
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
}

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 addresss must not exceed %d bytes", MaximumOwnerLength)
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
}

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

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 addresss must not exceed %d bytes", MaximumCounterpartyPayeeLength)
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
}

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
MaximumMemoLength = 32768 // maximum length of the memo in bytes
)

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 addresss must not exceed %d bytes", MaximumReceiverLength)
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
}
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