diff --git a/types/errors/errors.go b/types/errors/errors.go index 642c433d90b2..370e6c591469 100644 --- a/types/errors/errors.go +++ b/types/errors/errors.go @@ -65,6 +65,12 @@ var ( // ErrNoSignatures to doc ErrNoSignatures = Register(RootCodespace, 16, "no signatures supplied") + // ErrInvalidId to doc + ErrInvalidID = Register(RootCodespace, 17, "invalid identifier") + + // ErrInvalidPath to doc + ErrInvalidPath = Register(RootCodespace, 18, "invalid path") + // ErrPanic is only set when we recover from a panic, so we know to // redact potentially sensitive system info ErrPanic = Register(UndefinedCodespace, 111222, "panic") diff --git a/x/ibc/23-commitment/keeper/keeper.go b/x/ibc/23-commitment/keeper/keeper.go index 62790e6b83cb..eb1a2dfb8c60 100644 --- a/x/ibc/23-commitment/keeper/keeper.go +++ b/x/ibc/23-commitment/keeper/keeper.go @@ -45,7 +45,10 @@ func (k Keeper) BatchVerifyMembership(ctx sdk.Context, proof exported.ProofI, it continue } - path := types.ApplyPrefix(k.prefix, pathStr) + path, err := types.ApplyPrefix(k.prefix, pathStr) + if err != nil { + return false + } ok = proof.VerifyMembership(root, path, value) if !ok { return false @@ -69,7 +72,10 @@ func (k Keeper) BatchVerifyNonMembership(ctx sdk.Context, proof exported.ProofI, continue } - path := types.ApplyPrefix(k.prefix, pathStr) + path, err := types.ApplyPrefix(k.prefix, pathStr) + if err != nil { + return false + } ok = proof.VerifyNonMembership(root, path) if !ok { return false diff --git a/x/ibc/23-commitment/types/merkle.go b/x/ibc/23-commitment/types/merkle.go index 0a89c58e682b..730fb2496f3b 100644 --- a/x/ibc/23-commitment/types/merkle.go +++ b/x/ibc/23-commitment/types/merkle.go @@ -7,6 +7,7 @@ import ( "github.com/cosmos/cosmos-sdk/store/rootmulti" "github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/exported" + host "github.com/cosmos/cosmos-sdk/x/ibc/24-host" ) // ICS 023 Merkle Types Implementation @@ -103,13 +104,17 @@ func (p Path) String() string { // // CONTRACT: provided path string MUST be a well formated path. See ICS24 for // reference. -func ApplyPrefix(prefix exported.PrefixI, path string) Path { +func ApplyPrefix(prefix exported.PrefixI, path string) (Path, error) { + err := host.DefaultPathValidator(path) + if err != nil { + return Path{}, err + } // Split paths by the separator pathSlice := strings.Split(path, "/") commitmentPath := NewPath(pathSlice) commitmentPath.KeyPath = commitmentPath.KeyPath.AppendKey(prefix.Bytes(), merkle.KeyEncodingHex) - return commitmentPath + return commitmentPath, nil } var _ exported.ProofI = Proof{} diff --git a/x/ibc/24-host/errors.go b/x/ibc/24-host/errors.go new file mode 100644 index 000000000000..e1fa5e0e7a08 --- /dev/null +++ b/x/ibc/24-host/errors.go @@ -0,0 +1,16 @@ +package host + +import ( + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" +) + +// IBCCodeSpace is the codespace for all errors defined in the ibc module +const IBCCodeSpace = "ibc" + +var ( + // ErrInvalidID is returned if identifier string is invalid + ErrInvalidID = sdkerrors.Register(IBCCodeSpace, 1, "invalid identifier") + + // ErrInvalidPath is returned if path string is invalid + ErrInvalidPath = sdkerrors.Register(IBCCodeSpace, 2, "invalid path") +) diff --git a/x/ibc/24-host/validate.go b/x/ibc/24-host/validate.go new file mode 100644 index 000000000000..00dd5f8ba99e --- /dev/null +++ b/x/ibc/24-host/validate.go @@ -0,0 +1,74 @@ +package host + +import ( + "regexp" + "strings" + + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" +) + +// ICS 024 Identifier and Path Validation Implementation +// +// This file defines ValidateFn to validate identifier and path strings +// The spec for ICS 024 can be located here: +// https://github.com/cosmos/ics/tree/master/spec/ics-024-host-requirements + +// regular expression to check string is lowercase alphabetic characters only +var isAlphaLower = regexp.MustCompile(`^[a-z]+$`).MatchString + +// regular expression to check string is alphanumeric +var isAlphaNumeric = regexp.MustCompile(`^[a-zA-Z0-9]+$`).MatchString + +// ValidateFn function type to validate path and identifier bytestrings +type ValidateFn func(string) error + +// Default validator function for Client, Connection, and Channel +// identifiers +// Valid Identifier must be between 10-20 characters and only +// contain lowercase alphabetic characters +func DefaultIdentifierValidator(id string) error { + // valid id MUST NOT contain "/" separator + if strings.Contains(id, "/") { + return sdkerrors.Wrap(ErrInvalidID, "identifier cannot contain separator: /") + } + // valid id must be between 10 and 20 characters + if len(id) < 10 || len(id) > 20 { + return sdkerrors.Wrapf(ErrInvalidID, "identifier has invalid length: %d, must be between 10-20 characters", len(id)) + } + // valid id must contain only lower alphabetic characters + if !isAlphaLower(id) { + return sdkerrors.Wrap(ErrInvalidID, "identifier must contain only lowercase alphabetic characters") + } + return nil +} + +// NewPathValidator takes in a Identifier Validator function and returns +// a Path Validator function which requires path only has valid identifiers +// alphanumeric character strings, and "/" separators +func NewPathValidator(idValidator ValidateFn) ValidateFn { + return func(path string) error { + pathArr := strings.Split(path, "/") + for _, p := range pathArr { + // Each path element must either be valid identifier or alphanumeric + err := idValidator(p) + if err != nil && !isAlphaNumeric(p) { + return sdkerrors.Wrapf(ErrInvalidPath, "path contains invalid identifier or non-alphanumeric path element: %s", p) + } + } + return nil + } +} + +// Default Path Validator takes in path string and validates +// with default identifier rules. This is optimized by simply +// checking that all path elements are alphanumeric +func DefaultPathValidator(path string) error { + pathArr := strings.Split(path, "/") + for _, p := range pathArr { + // Each path element must either be alphanumeric + if !isAlphaNumeric(p) { + return sdkerrors.Wrapf(ErrInvalidPath, "invalid path element containing non-alphanumeric characters: %s", p) + } + } + return nil +}