Skip to content

Commit

Permalink
crypto/hd: make DerivePrivateKeyForPath error and not panic on traili…
Browse files Browse the repository at this point in the history
…ng slashes (#8607)

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 f970056)
  • Loading branch information
odeke-em authored and mergify-bot committed Feb 17, 2021
1 parent 421133b commit 9929a31
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 1 deletion.
9 changes: 8 additions & 1 deletion crypto/hd/hdpath.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"encoding/binary"
"fmt"
"math/big"
"path/filepath"
"strconv"
"strings"

Expand Down Expand Up @@ -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, "/")

Expand All @@ -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:
Expand Down
23 changes: 23 additions & 0 deletions crypto/hd/hdpath_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}

0 comments on commit 9929a31

Please sign in to comment.