Skip to content

Commit

Permalink
bip32: Only allow uppercase path elements with BIP32_FLAG_ALLOW_UPPER
Browse files Browse the repository at this point in the history
  • Loading branch information
jgriffiths committed Feb 10, 2022
1 parent 7e15609 commit 58f8251
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 8 deletions.
2 changes: 2 additions & 0 deletions include/wally_bip32.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ extern "C" {
#define BIP32_FLAG_STR_WILDCARD 0x8
/** Do not allow a leading ``m``/``M`` or ``/`` in path string expressions */
#define BIP32_FLAG_STR_BARE 0x10
/** Allow upper as well as lower case 'M'/'H' in path string expressions */
#define BIP32_FLAG_ALLOW_UPPER 0x20

/** Version codes for extended keys */
#define BIP32_VER_MAIN_PUBLIC 0x0488B21E
Expand Down
14 changes: 8 additions & 6 deletions src/bip32.c
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
BIP32_FLAG_SKIP_HASH | \
BIP32_FLAG_KEY_TWEAK_SUM | \
BIP32_FLAG_STR_WILDCARD | \
BIP32_FLAG_STR_BARE)
BIP32_FLAG_STR_BARE | \
BIP32_FLAG_ALLOW_UPPER)

static const unsigned char HMAC_KEY[] = {
'B', 'i', 't', 'c', 'o', 'i', 'n', ' ', 's', 'e', 'e', 'd'
Expand Down Expand Up @@ -85,16 +86,17 @@ static bool version_is_mainnet(uint32_t ver)
return ver == BIP32_VER_MAIN_PRIVATE || ver == BIP32_VER_MAIN_PUBLIC;
}

static bool is_hardened_indicator(char c)
static bool is_hardened_indicator(char c, bool allow_upper)
{
return c == '\'' || c == 'h' || c == 'H';
return c == '\'' || c == 'h' || (allow_upper && c == 'H');
}

static int path_from_string_n(const char *str, size_t str_len,
uint32_t child_num, uint32_t flags,
uint32_t *child_path, uint32_t child_path_len,
size_t *written)
{
const bool allow_upper = flags & BIP32_FLAG_ALLOW_UPPER;
size_t start, i = 0;
uint64_t v;

Expand All @@ -107,7 +109,7 @@ static int path_from_string_n(const char *str, size_t str_len,
if (i < str_len && str[i] == '/')
goto fail; /* bare path must start with a number */
} else {
if (i < str_len && (str[i] == 'm' || str[i] == 'M'))
if (i < str_len && (str[i] == 'm' || (allow_upper && str[i] == 'M')))
++i; /* Skip */
if (i < str_len && str[i] == '/')
++i; /* Skip */
Expand All @@ -127,7 +129,7 @@ static int path_from_string_n(const char *str, size_t str_len,
/* No number found */
if (str[i] == '/') {
if (i && (str[i - 1] < '0' || str[i - 1] > '9') &&
!is_hardened_indicator(str[i - 1]) && str[i - 1] != '*')
!is_hardened_indicator(str[i - 1], allow_upper) && str[i - 1] != '*')
goto fail; /* Only valid after number/wildcard/hardened indicator */
++i;
if (i == str_len || str[i] == '/')
Expand All @@ -147,7 +149,7 @@ static int path_from_string_n(const char *str, size_t str_len,
v = child_num; /* Use the given child number for the wildcard value */
}

if (is_hardened_indicator(str[i])) {
if (is_hardened_indicator(str[i], allow_upper)) {
v |= BIP32_INITIAL_HARDENED_CHILD;
++i;
}
Expand Down
4 changes: 4 additions & 0 deletions src/ctest/test_descriptor.c
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,10 @@ static const struct descriptor_err_test {
"descriptor errchk - invalid checksum",
"wpkh(02f9308a019258c31049344f85f89d5229b531c845836f99b08601f113bce036f9)#8rap84p2",
WALLY_NETWORK_BITCOIN_MAINNET
},{
"descriptor errchk - upper case hardened indicator",
"pkh(xprvA2YKGLieCs6cWCiczALiH1jzk3VCCS5M1pGQfWPkamCdR9UpBgE2Gb8AKAyVjKHkz8v37avcfRjdcnP19dVAmZrvZQfvTcXXSAiFNQ6tTtU/1H/2)",
WALLY_NETWORK_BITCOIN_MAINNET
},{
"descriptor errchk - privkey - unmatch network1",
"wpkh(cSMSHUGbEiZQUXVw9zA33yT3m8fgC27rn2XEGZJupwCpsRS3rAYa)",
Expand Down
15 changes: 13 additions & 2 deletions src/test/test_bip32.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@

FLAG_KEY_PRIVATE, FLAG_KEY_PUBLIC, FLAG_SKIP_HASH, = 0x0, 0x1, 0x2
FLAG_KEY_TWEAK_SUM, FLAG_STR_WILDCARD, FLAG_STR_BARE = 0x4, 0x8, 0x10
ALL_DEFINED_FLAGS = FLAG_KEY_PRIVATE | FLAG_KEY_PUBLIC | FLAG_SKIP_HASH
FLAG_ALLOW_UPPER = 0x20
ALL_DEFINED_FLAGS = FLAG_KEY_PRIVATE | FLAG_KEY_PUBLIC | FLAG_SKIP_HASH | \
FLAG_KEY_TWEAK_SUM | FLAG_STR_WILDCARD | FLAG_STR_BARE | FLAG_ALLOW_UPPER
BIP32_SERIALIZED_LEN = 78
BIP32_FLAG_SKIP_HASH = 0x2
EMPTY_PRIV_KEY = utf8('01' + ('00') * 32)
Expand Down Expand Up @@ -183,6 +185,13 @@ def derive_key_by_path(self, parent, path, flags, expected=WALLY_OK):
ret = bip32_key_from_parent_path_str(byref(parent), str_path, 0,
flags, byref(str_key_out))
self.assertEqual(ret, expected)
if expected == WALLY_OK:
# Verify that upper case is allowed with FLAG_ALLOW_UPPER
str_path = str_path.upper()
flags |= FLAG_ALLOW_UPPER
ret = bip32_key_from_parent_path_str(byref(parent), str_path, 0,
flags, byref(str_key_out))
self.assertEqual(ret, expected)
return key_out, str_key_out

def compare_keys(self, key, expected, flags):
Expand Down Expand Up @@ -493,7 +502,7 @@ def get_paths(path):
self.assertEqual(ret, WALLY_EINVAL)

c_path, str_path = get_paths(path_)
master.depth = 0xff # Cant derive from a parent of depth 255
master.depth = 0xff # Can't derive from a parent of depth 255
ret = bip32_key_from_parent(m, 5, FLAG_KEY_PUBLIC, key_out)
self.assertEqual(ret, WALLY_EINVAL)
ret = bip32_key_from_parent_path(m, c_path, len(c_path), FLAG_KEY_PUBLIC, key_out)
Expand All @@ -507,6 +516,8 @@ def get_paths(path):
cases = [('m', 0, 0), # Empty resulting path (1)
('m/', 0, 0), # Empty resulting path (2)
('/', 0, 0), # Empty resulting path (3)
('M/1', 0, 0), # Uppercase M without flag
('m/1H', 0, 0), # Uppercase H without flag
('//', 0, 0), # Trailing slash (1)
('/1/', 0, 0), # Trailing slash (2)
('m/1', B, 0), # Non-bare path (1)
Expand Down

0 comments on commit 58f8251

Please sign in to comment.