Skip to content

Commit cf28837

Browse files
author
MacroFake
committed
Merge bitcoin/bitcoin#26275: Fix crash on deriveaddresses when index is 2147483647 (2^31-1)
9153ff3 rpc: add non-regression test about deriveaddresses crash when index is 2147483647 (muxator) addf9d6 rpc: fix crash in deriveaddresses when derivation index is 2147483647 (muxator) Pull request description: This PR is a proposal for fixing #26274 (better described there). The problem is due to a signed int wrapping when the `index` parameter of the `deriveaddresses` RPC call has the value `2^31-1`. ```C++ for (int i = range_begin; i <= range_end; ++i) { ``` * the first commit adds a "temporary" test case (`test/functional/rpc_deriveaddresses_crash.py`) that shows the crash, and can be used to generate a core dump; * the second commit fixes the problem giving an explicit size to the `i` variable in a for loop, from `int` to `int64_t`. The same commit also removes the ephemeral test case and adds a passing test to `test/functional/rpc_deriveaddresses.py`, in order to prevent future regressions. This is my first submission to this project and I do not know its conventions. Please advise if something needs to be changed. ACKs for top commit: achow101: ACK 9153ff3 Tree-SHA512: 0477b57b15dc2c682cf539d6002f100d44a8c7e668041aa3340c39dcdbd40e083c75dec6896b6c076b044a01c2e5254272ae6696d8a1467539391926f270940a
2 parents 28cf756 + 9153ff3 commit cf28837

File tree

2 files changed

+8
-1
lines changed

2 files changed

+8
-1
lines changed

src/rpc/output_script.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ static RPCHelpMan deriveaddresses()
273273

274274
UniValue addresses(UniValue::VARR);
275275

276-
for (int i = range_begin; i <= range_end; ++i) {
276+
for (int64_t i = range_begin; i <= range_end; ++i) {
277277
FlatSigningProvider provider;
278278
std::vector<CScript> scripts;
279279
if (!desc->Expand(i, key_provider, scripts, provider)) {

test/functional/rpc_deriveaddresses.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,13 @@ def run_test(self):
4444
combo_descriptor = descsum_create("combo(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/0)")
4545
assert_equal(self.nodes[0].deriveaddresses(combo_descriptor), ["mtfUoUax9L4tzXARpw1oTGxWyoogp52KhJ", "mtfUoUax9L4tzXARpw1oTGxWyoogp52KhJ", address, "2NDvEwGfpEqJWfybzpKPHF2XH3jwoQV3D7x"])
4646

47+
# Before #26275, bitcoind would crash when deriveaddresses was
48+
# called with derivation index 2147483647, which is the maximum
49+
# positive value of a signed int32, and - currently - the
50+
# maximum value that the deriveaddresses bitcoin RPC call
51+
# accepts as derivation index.
52+
assert_equal(self.nodes[0].deriveaddresses(descsum_create("wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), [2147483647, 2147483647]), ["bcrt1qtzs23vgzpreks5gtygwxf8tv5rldxvvsyfpdkg"])
53+
4754
hardened_without_privkey_descriptor = descsum_create("wpkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1'/1/0)")
4855
assert_raises_rpc_error(-5, "Cannot derive script without private keys", self.nodes[0].deriveaddresses, hardened_without_privkey_descriptor)
4956

0 commit comments

Comments
 (0)