Skip to content

Test for lineage.depth overflow of the extended keys #1624

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

Test for lineage.depth overflow of the extended keys #1624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
188609e
Test for extended key depth overflow
Mar 31, 2025
b290f4f
Add failing tests for extended key depth overflow
Mar 31, 2025
1db5f42
Test for deriving public from invalid extended private key
Apr 1, 2025
44807d2
Test decoded null keys yields invalid keys
Apr 1, 2025
b4b1fb4
Guard deriving public from invalid extended private key
Apr 1, 2025
46ecf36
Remove accidentally commited dev env files
Apr 1, 2025
77aab49
Put tests into the correct files
Apr 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/wallet/keys/hd_private.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,11 @@ hd_key hd_private::to_hd_key() const NOEXCEPT

hd_public hd_private::to_public() const NOEXCEPT
{

if (!valid_) {
return {};
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid bracing for single line (also braces would be balanced).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like to be explicit about the if scope, as I have seen a new code added (maybe under the influence of Python-like, whitespace aware languages), without adding the braces. Tests should hopefully catch that though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/libbitcoin/libbitcoin-system/wiki/Style-Guide


return hd_public(((hd_public)*this).to_hd_key(),
hd_public::to_prefix(lineage_.prefixes));
}
Expand Down
108 changes: 108 additions & 0 deletions test/wallet/keys/hd_private.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,112 @@ BOOST_AUTO_TEST_CASE(hd_private__derive_public__long_seed__expected)
BOOST_REQUIRE_EQUAL(m0xH1yH2_pub.encoded(), "xpub6FnCn6nSzZAw5Tw7cgR9bi15UV96gLZhjDstkXXxvCLsUXBGXPdSnLFbdpq8p9HmGsApME5hQTZ3emM2rnY5agb9rXpVGyy3bdW6EEgAtqt");
}

BOOST_AUTO_TEST_CASE(hd_private__constructor__null_key_decodes_to_invalid__expected)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This:

hd_private__constructor__null_key_decodes_to_invalid__expected

would be better as:

hd_private__constructor__null_key__decodes_to_invalid

Others as well. Third part is the thing being tested and fourth part is the expectation.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, will improve.

{
// the 11...14rcJhr is a serialization of a null key;
static const auto null_encoded = "1111111111111111111111111111111111111111111111111111111111111111111111111111114rcJhr";
const hd_private xprv_null(null_encoded);

BOOST_REQUIRE(!xprv_null);
}

BOOST_AUTO_TEST_CASE(hd_private__to_public__fails_from_invalid_private__expected)
{
// the 11...14rcJhr is a serialization of a null key;
static const auto xprv_invalid_encoded = "1111111111111111111111111111111111111111111111111111111111111111111111111111114rcJhr";
const hd_private xprv_invalid(xprv_invalid_encoded);

BOOST_REQUIRE(!xprv_invalid);
BOOST_REQUIRE(!xprv_invalid.to_public());
}

BOOST_AUTO_TEST_CASE(hd_private__derive_private__must_not_overflow_depth__expected)
{
// xprv_254_depth was created from "xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi"
// by manually setting the depth to 254
static const auto xprv_254_encoded = "xprvJ6xRbBsatSpgzr9c3hYbM2RohnAcHiiN74vQWqdRPx914xeq41t3u4rPXTsNxd5kvLSnqpsMx1cMx8cytMM5RbS7G54nwC5p5P5MQFSjf36";
const hd_private xprv_254(xprv_254_encoded);


const auto xprv_255 = xprv_254.derive_private(14);
const auto xprv_256 = xprv_255.derive_private(70);

BOOST_REQUIRE_EQUAL(xprv_254.lineage().depth, 254);
BOOST_REQUIRE(xprv_254);
// the maximal valid depth is 255
BOOST_REQUIRE_EQUAL(xprv_255.lineage().depth, 255);
BOOST_REQUIRE(xprv_255);

// depth overflows uint from 255 to 0
BOOST_REQUIRE_EQUAL(xprv_256.lineage().depth, 0);
// which creates invalid keys
BOOST_REQUIRE(!xprv_256);
}

BOOST_AUTO_TEST_CASE(hd_private__derive_private__hardened_must_not_overflow_depth__expected)
{
// xprv_254_depth was created from "xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi"
// by manually setting the depth to 254
static const auto xprv254_encoded = "xprvJ6xRbBsatSpgzr9c3hYbM2RohnAcHiiN74vQWqdRPx914xeq41t3u4rPXTsNxd5kvLSnqpsMx1cMx8cytMM5RbS7G54nwC5p5P5MQFSjf36";
const hd_private xprv_254(xprv254_encoded);

const auto xprv_255 = xprv_254.derive_private(1337 + hd_first_hardened_key);
const auto xprv_256 = xprv_255.derive_private(8887 + hd_first_hardened_key);

BOOST_REQUIRE_EQUAL(xprv_254.lineage().depth, 254);
BOOST_REQUIRE(xprv_254);
// the maximal valid depth is 255
BOOST_REQUIRE_EQUAL(xprv_255.lineage().depth, 255);
BOOST_REQUIRE(xprv_255);

// depth overflows uint from 255 to 0
BOOST_REQUIRE_EQUAL(xprv_256.lineage().depth, 0);
// which creates invalid keys
BOOST_REQUIRE(!xprv_256);
}

BOOST_AUTO_TEST_CASE(hd_private__derive_public__must_not_overflow_depth__expected)
{
// xprv_254_depth was created from "xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi"
// by manually setting the depth to 254
static const auto xprv_254_encoded = "xprvJ6xRbBsatSpgzr9c3hYbM2RohnAcHiiN74vQWqdRPx914xeq41t3u4rPXTsNxd5kvLSnqpsMx1cMx8cytMM5RbS7G54nwC5p5P5MQFSjf36";
const hd_private xprv_254(xprv_254_encoded);

const auto xprv_255 = xprv_254.derive_private(14);
const auto xpub_256 = xprv_255.derive_public(70);

BOOST_REQUIRE_EQUAL(xprv_254.lineage().depth, 254);
BOOST_REQUIRE(xprv_254);
// the maximal valid depth is 255
BOOST_REQUIRE_EQUAL(xprv_255.lineage().depth, 255);
BOOST_REQUIRE(xprv_255);

// depth overflows uint from 255 to 0
BOOST_REQUIRE_EQUAL(xpub_256.lineage().depth, 0);
// which creates invalid keys
BOOST_REQUIRE(!xpub_256);
}

BOOST_AUTO_TEST_CASE(hd_private__derive_public__hardened_must_not_overflow_depth__expected)
{
// xprv_254_depth was created from "xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi"
// by manually setting the depth to 254
static const auto xprv_254_encoded = "xprvJ6xRbBsatSpgzr9c3hYbM2RohnAcHiiN74vQWqdRPx914xeq41t3u4rPXTsNxd5kvLSnqpsMx1cMx8cytMM5RbS7G54nwC5p5P5MQFSjf36";
const hd_private xprv_254(xprv_254_encoded);

const auto xprv_255 = xprv_254.derive_private(141);
const auto xpub_256 = xprv_255.derive_public(19287 + hd_first_hardened_key);

BOOST_REQUIRE_EQUAL(xprv_254.lineage().depth, 254);
BOOST_REQUIRE(xprv_254);
// the maximal valid depth is 255
BOOST_REQUIRE_EQUAL(xprv_255.lineage().depth, 255);
BOOST_REQUIRE(xprv_255);

// depth overflows uint from 255 to 0
BOOST_REQUIRE_EQUAL(xpub_256.lineage().depth, 0);
// which creates invalid keys
BOOST_REQUIRE(!xpub_256);
}

BOOST_AUTO_TEST_SUITE_END()
34 changes: 34 additions & 0 deletions test/wallet/keys/hd_public.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,16 @@ BOOST_AUTO_TEST_CASE(hd_public__encoded__round_trip__expected)
BOOST_REQUIRE_EQUAL(key.encoded(), encoded);
}

BOOST_AUTO_TEST_CASE(hd_public__constructor__null_key_decodes_to_invalid__expected)
{
// the 11...14rcJhr is a serialization of a null key;
static const auto null_encoded = "1111111111111111111111111111111111111111111111111111111111111111111111111111114rcJhr";
const hd_private xpub_null(null_encoded);

BOOST_REQUIRE(!xpub_null);
}


BOOST_AUTO_TEST_CASE(hd_public__derive_public__short_seed__expected)
{
data_chunk seed;
Expand Down Expand Up @@ -93,4 +103,28 @@ BOOST_AUTO_TEST_CASE(hd_public__derive_public__long_seed__expected)
BOOST_REQUIRE_EQUAL(m0xH1yH2_pub.encoded(), "xpub6FnCn6nSzZAw5Tw7cgR9bi15UV96gLZhjDstkXXxvCLsUXBGXPdSnLFbdpq8p9HmGsApME5hQTZ3emM2rnY5agb9rXpVGyy3bdW6EEgAtqt");
}

BOOST_AUTO_TEST_CASE(hd_public__derive_public__must_not_overflow_depth__expected)
{
// xprv_254_depth was created from "xprv9s21ZrQH143K3QTDL4LXw2F7HEK3wJUD2nW2nRk4stbPy6cq3jPPqjiChkVvvNKmPGJxWUtg6LnF5kejMRNNU3TGtRBeJgk33yuGBxrMPHi"
// by manually setting the depth to 254
static const auto xprv_254_encoded = "xprvJ6xRbBsatSpgzr9c3hYbM2RohnAcHiiN74vQWqdRPx914xeq41t3u4rPXTsNxd5kvLSnqpsMx1cMx8cytMM5RbS7G54nwC5p5P5MQFSjf36";
const hd_private xprv_254(xprv_254_encoded);
hd_public xpub_254 = xprv_254.to_public();

const auto xpub_255 = xpub_254.derive_public(1);
const auto xpub_256 = xpub_255.derive_public(0);

BOOST_REQUIRE_EQUAL(xpub_254.lineage().depth, 254);
BOOST_REQUIRE(xpub_254);
// the maximal valid depth is 255
BOOST_REQUIRE_EQUAL(xpub_255.lineage().depth, 255);
BOOST_REQUIRE(xpub_255);

// depth overflows uint from 255 to 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's actually overflowing here, it should be the default (invalid/null) key, and the depth value for that happens to be zero.

BOOST_REQUIRE_EQUAL(xpub_256.lineage().depth, 0);
// which creates invalid keys
BOOST_REQUIRE(!xpub_256);
}


BOOST_AUTO_TEST_SUITE_END()
Loading