imp: check length of slices of messages (#6256)

* imp: check length of slices of messages

* add changelog

* change test limits

CHANGELOG.md

@@ -58,6 +58,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### Improvements
 
 * (apps/27-interchain-accounts) [\#5533](https://github.com/cosmos/ibc-go/pull/5533) ICA host sets the host connection ID on `OnChanOpenTry`, so that ICA controller implementations are not obliged to set the value on `OnChanOpenInit` if they are not able.
+* (core/02-client, core/03-connection, apps/27-interchain-accounts) [\#6256](https://github.com/cosmos/ibc-go/pull/6256) Add length checking of array fields in messages.
 * (apps/27-interchain-accounts, apps/tranfer, apps/29-fee) [\#6253](https://github.com/cosmos/ibc-go/pull/6253) Allow channel handshake to succeed if fee middleware is wired up on one side, but not the other.
 
 ### Features

modules/apps/27-interchain-accounts/host/types/params.go

@@ -9,6 +9,8 @@ import (
 const (
 	// DefaultHostEnabled is the default value for the host param (set to true)
 	DefaultHostEnabled = true
+	// Maximum length of the allowlist
+	MaxAllowListLength = 500
 )
 
 // NewParams creates a new parameter configuration for the host submodule
@@ -30,6 +32,10 @@ func (p Params) Validate() error {
 }
 
 func validateAllowlist(allowMsgs []string) error {
+	if len(allowMsgs) > MaxAllowListLength {
+		return fmt.Errorf("allow list length must not exceed %d items", MaxAllowListLength)
+	}
+
 	if slices.Contains(allowMsgs, AllowAllHostMsgs) && len(allowMsgs) > 1 {
 		return fmt.Errorf("allow list must have only one element because the allow all host messages wildcard (%s) is present", AllowAllHostMsgs)
 	}

modules/apps/27-interchain-accounts/host/types/params_test.go

@@ -14,4 +14,5 @@ func TestValidateParams(t *testing.T) {
 	require.Error(t, types.NewParams(true, []string{""}).Validate())
 	require.Error(t, types.NewParams(true, []string{" "}).Validate())
 	require.Error(t, types.NewParams(true, []string{"*", "/cosmos.bank.v1beta1.MsgSend"}).Validate())
+	require.Error(t, types.NewParams(true, make([]string, types.MaxAllowListLength+1)).Validate())
 }

modules/core/02-client/types/params.go

@@ -6,6 +6,9 @@ import (
 	"strings"
 )
 
+// Maximum length of the allowed clients list
+const MaxAllowedClientsLength = 200
+
 // DefaultAllowedClients are the default clients for the AllowedClients parameter.
 // By default it allows all client types.
 var DefaultAllowedClients = []string{AllowAllClients}
@@ -46,6 +49,10 @@ func (p Params) IsAllowedClient(clientType string) bool {
 // validateClients checks that the given clients are not blank and there are no duplicates.
 // If AllowAllClients wildcard (*) is used, then there should no other client types in the allow list
 func validateClients(clients []string) error {
+	if len(clients) > MaxAllowedClientsLength {
+		return fmt.Errorf("allowed clients length must not exceed %d items", MaxAllowedClientsLength)
+	}
+
 	if slices.Contains(clients, AllowAllClients) && len(clients) > 1 {
 		return fmt.Errorf("allow list must have only one element because the allow all clients wildcard (%s) is present", AllowAllClients)
 	}

modules/core/02-client/types/params_test.go

@@ -40,6 +40,7 @@ func TestValidateParams(t *testing.T) {
 		{"blank client", NewParams(" "), false},
 		{"duplicate clients", NewParams(exported.Tendermint, exported.Tendermint), false},
 		{"allow all clients plus valid client", NewParams(AllowAllClients, exported.Tendermint), false},
+		{"too many allowed clients", NewParams(make([]string, MaxAllowedClientsLength+1)...), false},
 	}
 
 	for _, tc := range testCases {

modules/core/03-connection/types/msgs.go

@@ -129,6 +129,9 @@ func (msg MsgConnectionOpenTry) ValidateBasic() error {
 	if len(msg.CounterpartyVersions) == 0 {
 		return errorsmod.Wrap(ibcerrors.ErrInvalidVersion, "empty counterparty versions")
 	}
+	if len(msg.CounterpartyVersions) > MaxCounterpartyVersionsLength {
+		return errorsmod.Wrapf(ibcerrors.ErrInvalidVersion, "counterparty versions must not exceed %d items", MaxCounterpartyVersionsLength)
+	}
 	for i, version := range msg.CounterpartyVersions {
 		if err := ValidateVersion(version); err != nil {
 			return errorsmod.Wrapf(err, "basic validation failed on version with index %d", i)

modules/core/03-connection/types/msgs_test.go

@@ -159,8 +159,10 @@ func (suite *MsgTestSuite) TestNewMsgConnectionOpenTry() {
 		{"empty proofConsensus", types.NewMsgConnectionOpenTry("clienttotesta", "connectiontotest", "clienttotest", clientState, prefix, []*types.Version{ibctesting.ConnectionVersion}, 500, suite.proof, suite.proof, emptyProof, clientHeight, clientHeight, signer), false},
 		{"invalid consensusHeight", types.NewMsgConnectionOpenTry("clienttotesta", "connectiontotest", "clienttotest", clientState, prefix, []*types.Version{ibctesting.ConnectionVersion}, 500, suite.proof, suite.proof, suite.proof, clientHeight, clienttypes.ZeroHeight(), signer), false},
 		{"empty singer", types.NewMsgConnectionOpenTry("clienttotesta", "connectiontotest", "clienttotest", clientState, prefix, []*types.Version{ibctesting.ConnectionVersion}, 500, suite.proof, suite.proof, suite.proof, clientHeight, clientHeight, ""), false},
-		{"success", types.NewMsgConnectionOpenTry("clienttotesta", "connectiontotest", "clienttotest", clientState, prefix, []*types.Version{ibctesting.ConnectionVersion}, 500, suite.proof, suite.proof, suite.proof, clientHeight, clientHeight, signer), true},
 		{"invalid version", types.NewMsgConnectionOpenTry("clienttotesta", "connectiontotest", "clienttotest", clientState, prefix, []*types.Version{{}}, 500, suite.proof, suite.proof, suite.proof, clientHeight, clientHeight, signer), false},
+		{"too many counterparty versions", types.NewMsgConnectionOpenTry("clienttotesta", "connectiontotest", "clienttotest", clientState, prefix, make([]*types.Version, types.MaxCounterpartyVersionsLength+1), 500, suite.proof, suite.proof, suite.proof, clientHeight, clientHeight, signer), false},
+		{"too many features in counterparty version", types.NewMsgConnectionOpenTry("clienttotesta", "connectiontotest", "clienttotest", clientState, prefix, []*types.Version{{"v1", make([]string, types.MaxFeaturesLength+1)}}, 500, suite.proof, suite.proof, suite.proof, clientHeight, clientHeight, signer), false},
+		{"success", types.NewMsgConnectionOpenTry("clienttotesta", "connectiontotest", "clienttotest", clientState, prefix, []*types.Version{ibctesting.ConnectionVersion}, 500, suite.proof, suite.proof, suite.proof, clientHeight, clientHeight, signer), true},
 	}
 
 	for _, tc := range testCases {

modules/core/03-connection/types/version.go

@@ -27,6 +27,11 @@ var (
 	allowNilFeatureSet = map[string]bool{
 		DefaultIBCVersionIdentifier: false,
 	}
+
+	// MaxVersionsLength is the maximum number of versions that can be supported
+	MaxCounterpartyVersionsLength = 100
+	// MaxFeaturesLength is the maximum number of features that can be supported
+	MaxFeaturesLength = 100
 )
 
 // NewVersion returns a new instance of Version.
@@ -56,6 +61,9 @@ func ValidateVersion(version *Version) error {
 	if strings.TrimSpace(version.Identifier) == "" {
 		return errorsmod.Wrap(ErrInvalidVersion, "version identifier cannot be blank")
 	}
+	if len(version.Features) > MaxFeaturesLength {
+		return errorsmod.Wrapf(ErrInvalidVersion, "features length must not exceed %d items", MaxFeaturesLength)
+	}
 	for i, feature := range version.Features {
 		if strings.TrimSpace(feature) == "" {
 			return errorsmod.Wrapf(ErrInvalidVersion, "feature cannot be blank, index %d", i)