crypto/hd: properly catch index overflows to ensure conformance with BIP 32 (#7628)

* crypto/hd: properly catch index overflows to ensure conformance with BIP 32

Uses 31 bits as the bitsize argument to strconv.ParseUint to ensure
that we correctly parse values in the range [0, max(int32)]

Adds tests too to prevent future regressions of this subtlety.

Fixes #7627.

* Address Fedekunze's testing review
This commit is contained in:
Emmanuel T Odeke 2020-11-03 11:28:42 -08:00 committed by GitHub
parent 9f17bc77af
commit ec285f1798
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 66 additions and 1 deletions

View File

@ -173,7 +173,10 @@ func DerivePrivateKeyForPath(privKeyBytes, chainCode [32]byte, path string) ([]b
part = part[:len(part)-1]
}
idx, err := strconv.ParseUint(part, 10, 32)
// As per the extended keys specification in
// https://github.com/bitcoin/bips/blob/master/bip-0032.mediawiki#extended-keys
// index values are in the range [0, 1<<31-1] aka [0, max(int32)]
idx, err := strconv.ParseUint(part, 10, 31)
if err != nil {
return []byte{}, fmt.Errorf("invalid BIP 32 path: %s", err)
}

View File

@ -213,3 +213,65 @@ func TestCreateHDPath(t *testing.T) {
})
}
}
// Tests to ensure that any index value is in the range [0, max(int32)] as per
// the extended keys specification. If the index belongs to that of a hardened key,
// its 0x80000000 bit will be set, so we can still accept values in [0, max(int32)] and then
// increase its value as deriveKeyPath already augments.
// See issue https://github.com/cosmos/cosmos-sdk/issues/7627.
func TestDeriveHDPathRange(t *testing.T) {
seed := mnemonicToSeed("I am become Death, the destroyer of worlds!")
tests := []struct {
path string
wantErr string
}{
{
path: "1'/2147483648/0'/0/0",
wantErr: "out of range",
},
{
path: "2147483648'/1/0/0",
wantErr: "out of range",
},
{
path: "2147483648'/2147483648/0'/0/0",
wantErr: "out of range",
},
{
path: "1'/-5/0'/0/0",
wantErr: "invalid syntax",
},
{
path: "-2147483646'/1/0/0",
wantErr: "invalid syntax",
},
{
path: "-2147483648'/-2147483648/0'/0/0",
wantErr: "invalid syntax",
},
{
// Should pass.
path: "1'/2147483647/0'/0/0",
},
{
// Should pass.
path: "2147483647'/1/0'/0/0",
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.path, func(t *testing.T) {
master, ch := hd.ComputeMastersFromSeed(seed)
_, err := hd.DerivePrivateKeyForPath(master, ch, tt.path)
if tt.wantErr == "" {
require.Nil(t, err, "unexpected error")
} else {
require.NotNil(t, err, "expected a report of an int overflow")
require.Contains(t, err.Error(), tt.wantErr)
}
})
}
}