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

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
  • Loading branch information
odeke-em committed Feb 17, 2021
1 parent 56fc3fc commit a962bac
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 a962bac

Please sign in to comment.