Validate number of addresses in msg (CosmWasm#1926)

Co-authored-by: Christoph Otter <chris@confio.gmbh>

x/wasm/types/params.go

@@ -29,7 +29,7 @@ func (a AccessType) With(addrs ...sdk.AccAddress) AccessConfig {
 		for i, v := range addrs {
 			bech32Addrs[i] = v.String()
 		}
-		if err := assertValidAddresses(bech32Addrs); err != nil {
+		if err := validateBech32Addresses(bech32Addrs); err != nil {
 			panic(errorsmod.Wrap(err, "addresses"))
 		}
 		return AccessConfig{Permission: AccessTypeAnyOfAddresses, Addresses: bech32Addrs}
@@ -129,28 +129,11 @@ func (a AccessConfig) ValidateBasic() error {
 	case AccessTypeNobody, AccessTypeEverybody:
 		return nil
 	case AccessTypeAnyOfAddresses:
-		return errorsmod.Wrap(assertValidAddresses(a.Addresses), "addresses")
+		return errorsmod.Wrap(validateBech32Addresses(a.Addresses), "addresses")
 	}
 	return errorsmod.Wrapf(ErrInvalid, "unknown type: %q", a.Permission)
 }
 
-func assertValidAddresses(addrs []string) error {
-	if len(addrs) == 0 {
-		return ErrEmpty
-	}
-	idx := make(map[string]struct{}, len(addrs))
-	for _, a := range addrs {
-		if _, err := sdk.AccAddressFromBech32(a); err != nil {
-			return errorsmod.Wrapf(err, "address: %s", a)
-		}
-		if _, exists := idx[a]; exists {
-			return ErrDuplicate.Wrapf("address: %s", a)
-		}
-		idx[a] = struct{}{}
-	}
-	return nil
-}
-
 // Allowed returns if permission includes the actor.
 // Actor address must be valid and not nil
 func (a AccessConfig) Allowed(actor sdk.AccAddress) bool {

x/wasm/types/tx.go

@@ -12,6 +12,8 @@ import (
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
+const maxCodeIDCount = 50
+
 // RawContractMessage defines a json message that is sent or returned by a wasm contract.
 // This type can hold any type of bytes. Until validateBasic is called there should not be
 // any assumptions made that the data is valid syntax or semantic.
@@ -356,15 +358,14 @@ func (msg MsgPinCodes) ValidateBasic() error {
 	return validateCodeIDs(msg.CodeIDs)
 }
 
-const maxCodeIDTotal = 50
-
-// ensure not empty, not duplicates and not exceeding max number
+// validateCodeIDs ensures the list is not empty, has no duplicates
+// and does not exceed the max number of code IDs
 func validateCodeIDs(codeIDs []uint64) error {
 	switch n := len(codeIDs); {
 	case n == 0:
 		return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "empty code ids")
-	case n > maxCodeIDTotal:
-		return errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "total number of code ids is greater than %d", maxCodeIDTotal)
+	case n > maxCodeIDCount:
+		return errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "total number of code ids is greater than %d", maxCodeIDCount)
 	}
 	if hasDuplicates(codeIDs) {
 		return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "duplicate code ids")
@@ -468,11 +469,7 @@ func (msg MsgAddCodeUploadParamsAddresses) ValidateBasic() error {
 		return errorsmod.Wrap(err, "authority")
 	}
 
-	if len(msg.Addresses) == 0 {
-		return errorsmod.Wrap(ErrEmpty, "addresses")
-	}
-
-	return checkDuplicatedAddresses(msg.Addresses)
+	return validateBech32Addresses(msg.Addresses)
 }
 
 func (msg MsgRemoveCodeUploadParamsAddresses) Route() string {
@@ -488,26 +485,7 @@ func (msg MsgRemoveCodeUploadParamsAddresses) ValidateBasic() error {
 		return errorsmod.Wrap(err, "authority")
 	}
 
-	if len(msg.Addresses) == 0 {
-		return errorsmod.Wrap(ErrEmpty, "addresses")
-	}
-
-	return checkDuplicatedAddresses(msg.Addresses)
-}
-
-func checkDuplicatedAddresses(addresses []string) error {
-	index := map[string]struct{}{}
-	for _, addr := range addresses {
-		addr = strings.ToUpper(addr)
-		if _, err := sdk.AccAddressFromBech32(addr); err != nil {
-			return errorsmod.Wrap(err, "addresses")
-		}
-		if _, found := index[addr]; found {
-			return errorsmod.Wrap(ErrInvalid, "duplicate addresses")
-		}
-		index[addr] = struct{}{}
-	}
-	return nil
+	return validateBech32Addresses(msg.Addresses)
 }
 
 func (msg MsgStoreAndMigrateContract) Route() string {

x/wasm/types/tx_test.go

@@ -763,6 +763,13 @@ func TestMsgAddCodeUploadParamsAddressesValidation(t *testing.T) {
 	badAddress := "abcd"
 	// proper address size
 	goodAddress := sdk.AccAddress(make([]byte, 20)).String()
+	goodAddress2 := strings.ToUpper(goodAddress)
+	require.NotEqual(t, goodAddress, goodAddress2) // sanity check
+
+	tooManyAddresses := make([]string, MaxAddressCount+1)
+	for i := range tooManyAddresses {
+		tooManyAddresses[i] = sdk.AccAddress(bytes.Repeat([]byte{byte(i)}, 20)).String()
+	}
 
 	specs := map[string]struct {
 		src    MsgAddCodeUploadParamsAddresses
@@ -774,6 +781,12 @@ func TestMsgAddCodeUploadParamsAddressesValidation(t *testing.T) {
 				Addresses: []string{goodAddress},
 			},
 		},
+		"all good, uppercase": {
+			src: MsgAddCodeUploadParamsAddresses{
+				Authority: goodAddress,
+				Addresses: []string{goodAddress2},
+			},
+		},
 		"bad authority": {
 			src: MsgAddCodeUploadParamsAddresses{
 				Authority: badAddress,
@@ -807,6 +820,20 @@ func TestMsgAddCodeUploadParamsAddressesValidation(t *testing.T) {
 			},
 			expErr: true,
 		},
+		"duplicate addresses, different casing": {
+			src: MsgAddCodeUploadParamsAddresses{
+				Authority: goodAddress,
+				Addresses: []string{goodAddress, goodAddress2},
+			},
+			expErr: true,
+		},
+		"too many addresses": {
+			src: MsgAddCodeUploadParamsAddresses{
+				Authority: goodAddress,
+				Addresses: tooManyAddresses,
+			},
+			expErr: true,
+		},
 	}
 	for msg, spec := range specs {
 		t.Run(msg, func(t *testing.T) {
@@ -823,6 +850,13 @@ func TestMsgAddCodeUploadParamsAddressesValidation(t *testing.T) {
 func TestMsgRemoveCodeUploadParamsAddressesValidation(t *testing.T) {
 	// proper address size
 	goodAddress := sdk.AccAddress(make([]byte, 20)).String()
+	goodAddress2 := strings.ToUpper(goodAddress)
+	require.NotEqual(t, goodAddress, goodAddress2) // sanity check
+
+	tooManyAddresses := make([]string, MaxAddressCount+1)
+	for i := range tooManyAddresses {
+		tooManyAddresses[i] = sdk.AccAddress(bytes.Repeat([]byte{byte(i)}, 20)).String()
+	}
 
 	specs := map[string]struct {
 		src    MsgRemoveCodeUploadParamsAddresses
@@ -834,6 +868,12 @@ func TestMsgRemoveCodeUploadParamsAddressesValidation(t *testing.T) {
 				Addresses: []string{goodAddress},
 			},
 		},
+		"all good, uppercase": {
+			src: MsgRemoveCodeUploadParamsAddresses{
+				Authority: goodAddress,
+				Addresses: []string{goodAddress2},
+			},
+		},
 		"bad authority": {
 			src: MsgRemoveCodeUploadParamsAddresses{
 				Authority: badAddress,
@@ -867,6 +907,20 @@ func TestMsgRemoveCodeUploadParamsAddressesValidation(t *testing.T) {
 			},
 			expErr: true,
 		},
+		"duplicate addresses, different casing": {
+			src: MsgRemoveCodeUploadParamsAddresses{
+				Authority: goodAddress,
+				Addresses: []string{goodAddress, goodAddress2},
+			},
+			expErr: true,
+		},
+		"too many addresses": {
+			src: MsgRemoveCodeUploadParamsAddresses{
+				Authority: goodAddress,
+				Addresses: tooManyAddresses,
+			},
+			expErr: true,
+		},
 	}
 	for msg, spec := range specs {
 		t.Run(msg, func(t *testing.T) {

x/wasm/types/validation.go

@@ -9,6 +9,9 @@ import (
 	"github.com/distribution/reference"
 
 	errorsmod "cosmossdk.io/errors"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
 // MaxSaltSize is the longest salt that can be used when instantiating a contract
@@ -23,6 +26,9 @@ var (
 
 	// MaxProposalWasmSize is the largest a gov proposal compiled contract code can be when storing code on chain
 	MaxProposalWasmSize = 3 * 1024 * 1024 // extension point for chains to customize via compile flag.
+
+	// MaxAddressCount is the maximum number of addresses allowed within a message
+	MaxAddressCount = 50
 )
 
 func validateWasmCode(s []byte, maxSize int) error {
@@ -92,3 +98,29 @@ func ValidateVerificationInfo(source, builder string, codeHash []byte) error {
 	}
 	return nil
 }
+
+// validateBech32Addresses ensures the list is not empty, has no duplicates
+// and does not exceed the max number of addresses
+func validateBech32Addresses(addresses []string) error {
+	switch n := len(addresses); {
+	case n == 0:
+		return errorsmod.Wrap(ErrEmpty, "addresses")
+	case n > MaxAddressCount:
+		return errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "total number of addresses is greater than %d", MaxAddressCount)
+	}
+
+	index := map[string]struct{}{}
+	for _, addr := range addresses {
+		if _, err := sdk.AccAddressFromBech32(addr); err != nil {
+			return errorsmod.Wrapf(err, "address: %s", addr)
+		}
+		// Bech32 addresses are case-insensitive, i.e. the same address can have multiple representations,
+		// so we normalize here to avoid duplicates.
+		addr = strings.ToUpper(addr)
+		if _, found := index[addr]; found {
+			return errorsmod.Wrap(ErrDuplicate, "duplicate addresses")
+		}
+		index[addr] = struct{}{}
+	}
+	return nil
+}