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

Check length of unbounded string fields in messages (backport for v7) #1

Open
wants to merge 1 commit into
base: release/v7.4.0-classic
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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 (

var _ sdk.Msg = &MsgRegisterInterchainAccount{}

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

// NewMsgRegisterInterchainAccount creates a new instance of MsgRegisterInterchainAccount
func NewMsgRegisterInterchainAccount(connectionID, owner, version string) *MsgRegisterInterchainAccount {
return &MsgRegisterInterchainAccount{
Expand All @@ -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
}

Expand Down Expand Up @@ -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")
}
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 @@ -11,6 +11,8 @@ import (
host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
)

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

// msg types
const (
TypeMsgPayPacketFee = "payPacketFee"
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")
)
8 changes: 8 additions & 0 deletions modules/apps/transfer/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@

// msg types
const (
TypeMsgTransfer = "transfer"

Check failure on line 16 in modules/apps/transfer/types/msgs.go

View workflow job for this annotation

GitHub Actions / lint

File is not `gofumpt`-ed (gofumpt)
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 (
Expand Down Expand Up @@ -77,6 +79,12 @@
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
Loading