From d2b8fead56de9576059512a48dddb9e5ba5eaf2d Mon Sep 17 00:00:00 2001
From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com>
Date: Wed, 17 Feb 2021 11:05:22 +0000
Subject: [PATCH] crypto/hd: make DerivePrivateKeyForPath error and not panic
 on trailing slashes (#8607) (#8608)

Detected during my audit, right before fuzzing, the code that
checked for presence of hyphens per path segment assumed that
the part would always be non-empty. However, with paths such as:
* m/4/
* /44/
* m/4///

it'd panic with a runtime slice out of bounds.

With this new change, we now:
* firstly strip the right trailing slash
* on finding any empty segments of a path return an error

Fixes #8557

(cherry picked from commit f970056a92a77f38f9b15fd328568d8110946cc9)

Co-authored-by: Emmanuel T Odeke <emmanuel@orijtech.com>
---
 crypto/hd/hdpath.go      |  9 ++++++++-
 crypto/hd/hdpath_test.go | 23 +++++++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/crypto/hd/hdpath.go b/crypto/hd/hdpath.go
index 04469d730591..058ddcc45667 100644
--- a/crypto/hd/hdpath.go
+++ b/crypto/hd/hdpath.go
@@ -6,6 +6,7 @@ import (
 	"encoding/binary"
 	"fmt"
 	"math/big"
+	"path/filepath"
 	"strconv"
 	"strings"
 
@@ -177,6 +178,9 @@ func ComputeMastersFromSeed(seed []byte) (secret [32]byte, chainCode [32]byte) {
 // DerivePrivateKeyForPath derives the private key by following the BIP 32/44 path from privKeyBytes,
 // using the given chainCode.
 func DerivePrivateKeyForPath(privKeyBytes, chainCode [32]byte, path string) ([]byte, error) {
+	// First step is to trim the right end path separator lest we panic.
+	// See issue https://github.com/cosmos/cosmos-sdk/issues/8557
+	path = strings.TrimRightFunc(path, func(r rune) bool { return r == filepath.Separator })
 	data := privKeyBytes
 	parts := strings.Split(path, "/")
 
@@ -187,7 +191,10 @@ func DerivePrivateKeyForPath(privKeyBytes, chainCode [32]byte, path string) ([]b
 		parts = parts[1:]
 	}
 
-	for _, part := range parts {
+	for i, part := range parts {
+		if part == "" {
+			return nil, fmt.Errorf("path %q with split element #%d is an empty string", part, i)
+		}
 		// do we have an apostrophe?
 		harden := part[len(part)-1:] == "'"
 		// harden == private derivation, else public derivation:
diff --git a/crypto/hd/hdpath_test.go b/crypto/hd/hdpath_test.go
index 6dfda043eb04..8b1c40692c82 100644
--- a/crypto/hd/hdpath_test.go
+++ b/crypto/hd/hdpath_test.go
@@ -281,3 +281,26 @@ func ExampleSomeBIP32TestVecs() {
 	//
 	// c4c11d8c03625515905d7e89d25dfc66126fbc629ecca6db489a1a72fc4bda78
 }
+
+// Ensuring that we don't crash if values have trailing slashes
+// See issue https://github.com/cosmos/cosmos-sdk/issues/8557.
+func TestDerivePrivateKeyForPathDoNotCrash(t *testing.T) {
+	paths := []string{
+		"m/5/",
+		"m/5",
+		"/44",
+		"m//5",
+		"m/0/7",
+		"/",
+		" m       /0/7",          // Test case from fuzzer
+		"              /       ", // Test case from fuzzer
+		"m///7//////",
+	}
+
+	for _, path := range paths {
+		path := path
+		t.Run(path, func(t *testing.T) {
+			hd.DerivePrivateKeyForPath([32]byte{}, [32]byte{}, path)
+		})
+	}
+}