From ec285f17983ae87425dae0a636d67fd81c0a10da Mon Sep 17 00:00:00 2001
From: Emmanuel T Odeke <emmanuel@orijtech.com>
Date: Tue, 3 Nov 2020 11:28:42 -0800
Subject: [PATCH] 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
---
 crypto/hd/hdpath.go      |  5 +++-
 crypto/hd/hdpath_test.go | 62 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/crypto/hd/hdpath.go b/crypto/hd/hdpath.go
index ac00c4a51e..67a02f0290 100644
--- a/crypto/hd/hdpath.go
+++ b/crypto/hd/hdpath.go
@@ -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)
 		}
diff --git a/crypto/hd/hdpath_test.go b/crypto/hd/hdpath_test.go
index 9f95f7edec..42622b9a9c 100644
--- a/crypto/hd/hdpath_test.go
+++ b/crypto/hd/hdpath_test.go
@@ -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)
+			}
+		})
+	}
+}