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

* imp: check length of slices of messages

* add changelog

* change test limits

(cherry picked from commit 478f4c6)

# Conflicts:
#	CHANGELOG.md
#	modules/apps/27-interchain-accounts/host/types/params.go
#	modules/apps/27-interchain-accounts/host/types/params_test.go
#	modules/core/02-client/types/params.go
#	modules/core/02-client/types/params_test.go

CHANGELOG.md

@@ -46,8 +46,13 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+<<<<<<< HEAD
 * (apps/27-interchain-accounts) [\#6147](https://github.com/cosmos/ibc-go/pull/6147) Emit an event signalling that the host submodule is disabled.
 * (testing) [\#6180](https://github.com/cosmos/ibc-go/pull/6180) Add version to tm abci headers in ibctesting.
+=======
+* (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.
+>>>>>>> 478f4c60 (imp: check length of slices of messages (#6256))
 * (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

@@ -10,6 +10,8 @@ import (
 const (
 	// DefaultHostEnabled is the default value for the host param (set to true)
 	DefaultHostEnabled = true
+	// Maximum length of the allowlist
+	MaxAllowListLength = 500
 )
 
 var (
@@ -50,6 +52,7 @@ func (p Params) Validate() error {
 	return nil
 }
 
+<<<<<<< HEAD
 // ParamSetPairs implements params.ParamSet
 func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs {
 	return paramtypes.ParamSetPairs{
@@ -71,6 +74,15 @@ func validateAllowlist(i interface{}) error {
 	allowMsgs, ok := i.([]string)
 	if !ok {
 		return fmt.Errorf("invalid parameter type: %T", i)
+=======
+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)
+>>>>>>> 478f4c60 (imp: check length of slices of messages (#6256))
 	}
 
 	for _, typeURL := range allowMsgs {

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

@@ -11,4 +11,11 @@ import (
 func TestValidateParams(t *testing.T) {
 	require.NoError(t, types.DefaultParams().Validate())
 	require.NoError(t, types.NewParams(false, []string{}).Validate())
+<<<<<<< HEAD
+=======
+	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())
+>>>>>>> 478f4c60 (imp: check length of slices of messages (#6256))
 }

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

@@ -9,6 +9,7 @@ import (
 	"github.com/cosmos/ibc-go/v7/modules/core/exported"
 )
 
+<<<<<<< HEAD
 var (
 	// DefaultAllowedClients are the default clients for the AllowedClients parameter.
 	DefaultAllowedClients = []string{exported.Solomachine, exported.Tendermint, exported.Localhost}
@@ -21,6 +22,14 @@ var (
 func ParamKeyTable() paramtypes.KeyTable {
 	return paramtypes.NewKeyTable().RegisterParamSet(&Params{})
 }
+=======
+// 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}
+>>>>>>> 478f4c60 (imp: check length of slices of messages (#6256))
 
 // NewParams creates a new parameter configuration for the ibc client module
 func NewParams(allowedClients ...string) Params {
@@ -46,6 +55,7 @@ func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs {
 	}
 }
 
+<<<<<<< HEAD
 // IsAllowedClient checks if the given client type is registered on the allowlist.
 func (p Params) IsAllowedClient(clientType string) bool {
 	for _, allowedClient := range p.AllowedClients {
@@ -60,6 +70,17 @@ func validateClients(i interface{}) error {
 	clients, ok := i.([]string)
 	if !ok {
 		return fmt.Errorf("invalid parameter type: %T", i)
+=======
+// 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)
+>>>>>>> 478f4c60 (imp: check length of slices of messages (#6256))
 	}
 
 	for i, clientType := range clients {

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

@@ -17,6 +17,12 @@ func TestValidateParams(t *testing.T) {
 		{"default params", DefaultParams(), true},
 		{"custom params", NewParams(exported.Tendermint), true},
 		{"blank client", NewParams(" "), false},
+<<<<<<< HEAD
+=======
+		{"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},
+>>>>>>> 478f4c60 (imp: check length of slices of messages (#6256))
 	}
 
 	for _, tc := range testCases {

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

@@ -133,6 +133,9 @@ func (msg MsgConnectionOpenTry) ValidateBasic() error {
 	if len(msg.CounterpartyVersions) == 0 {
 		return sdkerrors.Wrap(sdkerrors.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 sdkerrors.Wrapf(err, "basic validation failed on version with index %d", i)

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

@@ -150,8 +150,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

@@ -28,6 +28,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
 )
 
 var _ exported.Version = &Version{}
@@ -59,6 +64,9 @@ func ValidateVersion(version *Version) error {
 	if strings.TrimSpace(version.Identifier) == "" {
 		return sdkerrors.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 sdkerrors.Wrapf(ErrInvalidVersion, "feature cannot be blank, index %d", i)