diff --git a/x/wasm/types/params.go b/x/wasm/types/params.go index 4b923b1bd3..c5cfb67229 100644 --- a/x/wasm/types/params.go +++ b/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 { diff --git a/x/wasm/types/tx.go b/x/wasm/types/tx.go index ecd68bddf4..057f77ff0b 100644 --- a/x/wasm/types/tx.go +++ b/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 { diff --git a/x/wasm/types/tx_test.go b/x/wasm/types/tx_test.go index 1045b95028..6f237b11a6 100644 --- a/x/wasm/types/tx_test.go +++ b/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) { diff --git a/x/wasm/types/validation.go b/x/wasm/types/validation.go index bd67e3bc26..f10de3d410 100644 --- a/x/wasm/types/validation.go +++ b/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 +}