ICS 24 Implementation (#5229)

* add validation functions

* validate path in ics-23

* address @fede comments

* move errors into host package

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")

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

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{}

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")
+)

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
+}