crypto/hd: DerivePrivateKeyForPath can panic with index out of bounds if the path ends with a slash "/", due to improper check of length

[odeke-em]

Summary of Bug

Found by auditing the code for Stargate and preparing to fuzz it, if any path is provided with a trailing slash for example just by simply passing in "m/16/19/" instead of "m/16/19" or even "/", or "m/16//10"

func TestDerivePrivateKeyForPath(t *testing.T) {
        hd.DerivePrivateKeyForPath([32]byte{}, [32]byte{}, "m/5/")
}

results in this panic

--- FAIL: TestDerivePrivateKeyForPath (0.00s)
panic: runtime error: slice bounds out of range [-1:] [recovered]
	panic: runtime error: slice bounds out of range [-1:]

goroutine 36 [running]:
testing.tRunner.func1.2(0x18b9080, 0xc00024bea8)
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/testing.go:1144 +0x332
testing.tRunner.func1(0xc000103080)
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/testing.go:1147 +0x4b6
panic(0x18b9080, 0xc00024bea8)
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/runtime/panic.go:965 +0x1b9
github.com/cosmos/cosmos-sdk/crypto/hd.DerivePrivateKeyForPath(0x0, 0x0, 0x0, 0x0, 0xed2c7f8abb74f3c9, 0x8f0215d93407b907, 0x7a28ae7c374e8027, 0xb869ce287cabd893, 0x191cada, 0x4, ...)
	/Users/emmanuelodeke/go/src/github.com/cosmos/cosmos-sdk/crypto/hd/hdpath.go:192 +0x587
github.com/cosmos/cosmos-sdk/crypto/hd_test.TestDerivePrivateKeyForPath(0xc000103080)
	/Users/emmanuelodeke/go/src/github.com/cosmos/cosmos-sdk/crypto/hd/hdpath_test.go:286 +0x4d
testing.tRunner(0xc000103080, 0x1a80808)
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/testing.go:1194 +0xef
created by testing.(*T).Run
	/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/testing.go:1239 +0x2b3
exit status 2
FAIL	github.com/cosmos/cosmos-sdk/crypto/hd	0.296s

Version

All versions with this code

Cause

This code

cosmos-sdk/crypto/hd/hdpath.go

Lines 190 to 192 in c54025d

	for _, part := range parts {
	// do we have an apostrophe?
	harden := part[len(part)-1:] == "'"

assumes that ALL the parts will have a non-empty string, but that's clearly not true, if a path with a trailing slash were passed it, it'd crash.

Remedy

Before that part[len(part)-1] == "'", we need to check that the segment is non-empty. There is 2 fold remedy:

• replace "//" with "/"
• strip trailing and starting slashes

Let's please backport this change to all releases like Stargate et al.

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned