From fa4296a40ca75d994210cf88aca2208c3e49ff14 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Mon, 16 Oct 2023 14:49:24 +0200 Subject: [PATCH] add check for length of version and counterparty version in channel handshake messages --- modules/core/04-channel/types/channel.go | 5 +++++ modules/core/04-channel/types/msgs.go | 6 ++++++ modules/core/04-channel/types/msgs_test.go | 6 ++++++ 3 files changed, 17 insertions(+) diff --git a/modules/core/04-channel/types/channel.go b/modules/core/04-channel/types/channel.go index d288a2a8842..ba1da3e8b99 100644 --- a/modules/core/04-channel/types/channel.go +++ b/modules/core/04-channel/types/channel.go @@ -7,6 +7,8 @@ import ( "github.com/cosmos/ibc-go/v8/modules/core/exported" ) +const MaximumVersionLength = 8192 // maximum length of the version in bytes + var ( _ exported.ChannelI = (*Channel)(nil) _ exported.CounterpartyChannelI = (*Counterparty)(nil) @@ -78,6 +80,9 @@ func (ch Channel) ValidateBasic() error { if err := host.ConnectionIdentifierValidator(ch.ConnectionHops[0]); err != nil { return errorsmod.Wrap(err, "invalid connection hop ID") } + if len(ch.Version) > MaximumVersionLength { + return errorsmod.Wrapf(ErrInvalidChannelVersion, "version must not exceed %d bytes", MaximumVersionLength) + } return ch.Counterparty.ValidateBasic() } diff --git a/modules/core/04-channel/types/msgs.go b/modules/core/04-channel/types/msgs.go index 812829307c0..c0cd1159120 100644 --- a/modules/core/04-channel/types/msgs.go +++ b/modules/core/04-channel/types/msgs.go @@ -123,6 +123,9 @@ func (msg MsgChannelOpenTry) ValidateBasic() error { if err := host.ChannelIdentifierValidator(msg.Channel.Counterparty.ChannelId); err != nil { return errorsmod.Wrap(err, "invalid counterparty channel ID") } + if len(msg.CounterpartyVersion) > MaximumVersionLength { + return errorsmod.Wrapf(ErrInvalidChannelVersion, "counterparty version must not exceed %d bytes", MaximumVersionLength) + } _, err := sdk.AccAddressFromBech32(msg.Signer) if err != nil { @@ -170,6 +173,9 @@ func (msg MsgChannelOpenAck) ValidateBasic() error { if len(msg.ProofTry) == 0 { return errorsmod.Wrap(commitmenttypes.ErrInvalidProof, "cannot submit an empty try proof") } + if len(msg.CounterpartyVersion) > MaximumVersionLength { + return errorsmod.Wrapf(ErrInvalidChannelVersion, "counterparty version must not exceed %d bytes", MaximumVersionLength) + } _, err := sdk.AccAddressFromBech32(msg.Signer) if err != nil { return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err) diff --git a/modules/core/04-channel/types/msgs_test.go b/modules/core/04-channel/types/msgs_test.go index 8570ad59065..aa0c47d6c56 100644 --- a/modules/core/04-channel/types/msgs_test.go +++ b/modules/core/04-channel/types/msgs_test.go @@ -18,6 +18,7 @@ import ( clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" commitmenttypes "github.com/cosmos/ibc-go/v8/modules/core/23-commitment/types" + ibctesting "github.com/cosmos/ibc-go/v8/testing" "github.com/cosmos/ibc-go/v8/testing/simapp" ) @@ -125,6 +126,7 @@ func (suite *TypesTestSuite) TestMsgChannelOpenInitValidateBasic() { {"too short connection id", types.NewMsgChannelOpenInit(portid, version, types.UNORDERED, invalidShortConnHops, cpportid, addr), false}, {"too long connection id", types.NewMsgChannelOpenInit(portid, version, types.UNORDERED, invalidLongConnHops, cpportid, addr), false}, {"connection id contains non-alpha", types.NewMsgChannelOpenInit(portid, version, types.UNORDERED, []string{invalidConnection}, cpportid, addr), false}, + {"too long version", types.NewMsgChannelOpenInit(portid, ibctesting.GenerateString(types.MaximumVersionLength+1), types.UNORDERED, connHops, cpportid, addr), false}, {"", types.NewMsgChannelOpenInit(portid, "", types.UNORDERED, connHops, cpportid, addr), true}, {"invalid counterparty port id", types.NewMsgChannelOpenInit(portid, version, types.UNORDERED, connHops, invalidPort, addr), false}, {"channel not in INIT state", &types.MsgChannelOpenInit{portid, tryOpenChannel, addr}, false}, @@ -162,7 +164,10 @@ func (suite *TypesTestSuite) TestMsgChannelOpenTryValidateBasic() { {"connection hops more than 1 ", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, invalidConnHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, {"too short connection id", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, invalidShortConnHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, {"too long connection id", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, invalidLongConnHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, + {"too long connection id", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, invalidLongConnHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, {"connection id contains non-alpha", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, []string{invalidConnection}, cpportid, cpchanid, version, suite.proof, height, addr), false}, + {"too long counterparty version", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, connHops, cpportid, cpchanid, ibctesting.GenerateString(types.MaximumVersionLength+1), suite.proof, height, addr), false}, + {"too long version", types.NewMsgChannelOpenTry(portid, ibctesting.GenerateString(types.MaximumVersionLength+1), types.UNORDERED, connHops, cpportid, cpchanid, version, suite.proof, height, addr), false}, {"", types.NewMsgChannelOpenTry(portid, "", types.UNORDERED, connHops, cpportid, cpchanid, version, suite.proof, height, addr), true}, {"invalid counterparty port id", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, connHops, invalidPort, cpchanid, version, suite.proof, height, addr), false}, {"invalid counterparty channel id", types.NewMsgChannelOpenTry(portid, version, types.UNORDERED, connHops, cpportid, invalidChannel, version, suite.proof, height, addr), false}, @@ -199,6 +204,7 @@ func (suite *TypesTestSuite) TestMsgChannelOpenAckValidateBasic() { {"too short channel id", types.NewMsgChannelOpenAck(portid, invalidShortChannel, chanid, version, suite.proof, height, addr), false}, {"too long channel id", types.NewMsgChannelOpenAck(portid, invalidLongChannel, chanid, version, suite.proof, height, addr), false}, {"channel id contains non-alpha", types.NewMsgChannelOpenAck(portid, invalidChannel, chanid, version, suite.proof, height, addr), false}, + {"too long counterparty version", types.NewMsgChannelOpenAck(portid, chanid, chanid, ibctesting.GenerateString(types.MaximumVersionLength+1), suite.proof, height, addr), false}, {"", types.NewMsgChannelOpenAck(portid, chanid, chanid, "", suite.proof, height, addr), true}, {"empty proof", types.NewMsgChannelOpenAck(portid, chanid, chanid, version, emptyProof, height, addr), false}, {"invalid counterparty channel id", types.NewMsgChannelOpenAck(portid, chanid, invalidShortChannel, version, suite.proof, height, addr), false},