fix: update IsRevisionFormat and IsClientIDFormat to account for newl…

…ines before the dash (#724)

* update IsRevisionFormat to disallow newlines before the -<revision-number>

* update IsClientIDFormat to account for newlines before the dash

* add changelog

* Update modules/core/02-client/types/height.go

* add test case and update CHANGELOG

CHANGELOG.md

@@ -59,6 +59,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (modules/core/05-port) [\#288](https://github.com/cosmos/ibc-go/issues/288) Making the 05-port keeper function IsBound public. The IsBound function checks if the provided portID is already binded to a module.
 * (channel) [\#644](https://github.com/cosmos/ibc-go/pull/644) Adds `GetChannelConnection` to the ChannelKeeper. This function returns the connectionID and connection state associated with a channel. 
 * (channel) [\647](https://github.com/cosmos/ibc-go/pull/647) Reorganizes channel handshake handling to set channel state after IBC application callbacks. 
+* (client) [\#724](https://github.com/cosmos/ibc-go/pull/724) `IsRevisionFormat` and `IsClientIDFormat` have been updated to disallow newlines before the dash used to separate the chainID and revision number, and the client type and client sequence. 
 
 ### Features
 

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

@@ -16,9 +16,9 @@ import (
 var _ exported.Height = (*Height)(nil)
 
 // IsRevisionFormat checks if a chainID is in the format required for parsing revisions
-// The chainID must be in the form: `{chainID}-{revision}
+// The chainID must be in the form: `{chainID}-{revision}`.
 // 24-host may enforce stricter checks on chainID
-var IsRevisionFormat = regexp.MustCompile(`^.*[^-]-{1}[1-9][0-9]*$`).MatchString
+var IsRevisionFormat = regexp.MustCompile(`^.*[^\n-]-{1}[1-9][0-9]*$`).MatchString
 
 // ZeroHeight is a helper function which returns an uninitialized height.
 func ZeroHeight() Height {

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

@@ -112,16 +112,19 @@ func TestParseChainID(t *testing.T) {
 		{"gaiamainnet--4", 0, false},
 		{"gaiamainnet-3.4", 0, false},
 		{"gaiamainnet", 0, false},
+		{"gaiamain\nnet-1", 0, false}, // newlines not allowed in chainID
+		{"gaiamainnet-1\n", 0, false}, // newlines not allowed after dash
+		{"gaiamainnet\n-3", 0, false}, // newlines not allowed before revision number
 		{"a--1", 0, false},
 		{"-1", 0, false},
 		{"--1", 0, false},
 	}
 
-	for i, tc := range cases {
+	for _, tc := range cases {
 		require.Equal(t, tc.formatted, types.IsRevisionFormat(tc.chainID), "id %s does not match expected format", tc.chainID)
 
 		revision := types.ParseChainID(tc.chainID)
-		require.Equal(t, tc.revision, revision, "case %d returns incorrect revision", i)
+		require.Equal(t, tc.revision, revision, "chainID %s returns incorrect revision", tc.chainID)
 	}
 
 }

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

@@ -34,7 +34,7 @@ func FormatClientIdentifier(clientType string, sequence uint64) string {
 
 // IsClientIDFormat checks if a clientID is in the format required on the SDK for
 // parsing client identifiers. The client identifier must be in the form: `{client-type}-{N}
-var IsClientIDFormat = regexp.MustCompile(`^.*[^-]-[0-9]{1,20}$`).MatchString
+var IsClientIDFormat = regexp.MustCompile(`^.*[^\n-]-[0-9]{1,20}$`).MatchString
 
 // IsValidClientID checks if the clientID is valid and can be parsed into the client
 // identifier format.

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

@@ -27,6 +27,8 @@ func TestParseClientIdentifier(t *testing.T) {
 		{"invalid uint64", "tendermint-18446744073709551616", "tendermint", 0, false},
 		// uint64 == 20 characters
 		{"invalid large sequence", "tendermint-2345682193567182931243", "tendermint", 0, false},
+		{"invalid newline in clientID", "tendermin\nt-1", "tendermin\nt", 0, false},
+		{"invalid newline character before dash", "tendermint\n-1", "tendermint", 0, false},
 		{"missing dash", "tendermint0", "tendermint", 0, false},
 		{"blank id", "               ", "    ", 0, false},
 		{"empty id", "", "", 0, false},