Repository: bitcoin/bitcoin (language: cpp)

Issue:
bitcoind dumps core when deriveaddresses is called with index 2147483647 (2^31-1)
The `deriveaddresses` JSON-RPC endpoint accepts a descriptor and an index for the derivation.

Currently, the RPC interface caps the allowed index derivation range in the interval [0,2^31-1].

However, calling `deriveaddresses` using a range that includes the index `2147483647` (2^31-1) results in a crash of bitcoind (on an AMD64 machine):

```
bitcoin-cli deriveaddresses "descriptor" "[2147483647, 2147483647]"

[bitcoind dumps core]
```

In the PR that addresses this pull request (#26275) is included a test case that causes the crash.
For convenience, here's the contents of a core dump generated with that test:

```
$ sudo coredumpctl debug bitcoind
           PID: 110184 (bitcoind)
           UID: 1000 (muxator)
           GID: 1000 (muxator)
        Signal: 6 (ABRT)
     Timestamp: Thu 2022-10-06 17:48:08 CEST (4min 56s ago)
  Command Line: <base>/src/bitcoind -datadir=/tmp/test_runner_₿_🏃_20221006_174805/rpc_deriveaddresses_crash_0/node0 -logtimemicros -debug -debugexclude=libevent -debugexclude=leveldb -uacomment=testnode0 -logthreadnames -logsourcelocations -loglevel=trace -sandbox=log-and-abort
    Executable: <base>/src/bitcoind
[...]
    Message: Process 110184 (bitcoind) of user 1000 dumped core.

[...]
                ELF object binary architecture: AMD x86-64

GNU gdb (GDB) Fedora 12.1-1.fc36
[...]
Core was generated by `<base>/src/bitcoind -datadir=/tmp/test_runner_₿_��'.
Program terminated with signal SIGABRT, Aborted.
#0  0x00007f55a8ac6c4c in __pthread_kill_implementation () from /lib64/libc.so.6
[Current thread is 1 (Thread 0x7f5576ffd640 (LWP 110197))]
Missing separate debuginfos, use: dnf debuginfo-install glibc-2.35-17.fc36.x86_64 libevent-2.1.12-6.fc36.x86_64 libgcc-12.2.1-2.fc36.x86_64 libstdc++-12.2.1-2.fc36.x86_64 sqlite-libs-3.36.0-5.fc36.x86_64 zlib-1.2.11-33.fc36.x86_64
(gdb) bt
#0  0x00007f55a8ac6c4c in __pthread_kill_implementation () from /lib64/libc.so.6
#1  0x00007f55a8a769c6 in raise () from /lib64/libc.so.6
#2  0x00007f55a8a607f4 in abort () from /lib64/libc.so.6
#3  0x00007f55a8c40c11 in __addvsi3 () from /lib64/libgcc_s.so.1
#4  0x0000563935a84d73 in operator() (__closure=0x7f5576ffbd50, self=..., request=...) at rpc/output_script.cpp:276
#5  0x0000563935a87ec2 in std::__invoke_impl<UniValue, deriveaddresses()::<lambda(const RPCHelpMan&, const JSONRPCRequest&)>&, const RPCHelpMan&, const JSONRPCRequest&>(std::__invoke_other, struct {...} &) (__f=...)
    at /usr/include/c++/12/bits/invoke.h:61
#6  0x0000563935a8773f in std::__invoke_r<UniValue, deriveaddresses()::<lambda(const RPCHelpMan&, const JSONRPCRequest&)>&, const RPCHelpMan&, const JSONRPCRequest&>(struct {...} &) (__fn=...) at /usr/include/c++/12/bits/invoke.h:116
#7  0x0000563935a86ec3 in std::_Function_handler<UniValue(const RPCHelpMan&, const JSONRPCRequest&), deriveaddresses()::<lambda(const RPCHelpMan&, const JSONRPCRequest&)> >::_M_invoke(const std::_Any_data &, const RPCHelpMan &, const JSONRPCRequest &) (__functor=..., __args#0=..., __args#1=...) at /usr/include/c++/12/bits/std_function.h:291
#8  0x00005639360b4641 in std::function<UniValue (RPCHelpMan const&, JSONRPCRequest const&)>::operator()(RPCHelpMan const&, JSONRPCRequest const&) const (this=0x7f5576ffbd50, __args#0=..., __args#1=...)
    at /usr/include/c++/12/bits/std_function.h:591
#9  0x00005639360a93b8 in RPCHelpMan::HandleRequest (this=0x7f5576ffbd30, request=...) at rpc/util.cpp:585
#10 0x00005639359cc1a2 in CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)())::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}::operator()(JSONRPCRequest const&, UniValue&, bool) const (__closure=0x5639368fbb00 <RegisterOutputScriptRPCCommands(CRPCTable&)::commands+320>, request=..., result=...) at ./rpc/server.h:109
#11 0x00005639359e0be3 in std::__invoke_impl<bool, CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)())::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}&, JSONRPCRequest const&, UniValue&, bool>(std::__invoke_other, CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)())::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}&, JSONRPCRequest const&, UniValue&, bool&&) (__f=...) at /usr/include/c++/12/bits/invoke.h:61
#12 0x00005639359da9ba in std::__invoke_r<bool, CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)())::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}&, JSONRPCRequest const&, UniValue&, bool>(CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)())::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}&, JSONRPCRequest const&, UniValue&, bool&&) (__fn=...) at /usr/include/c++/12/bits/invoke.h:114
#13 0x00005639359d4916 in std::_Function_handler<bool (JSONRPCRequest const&, UniValue&, bool), CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)())::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}>::_M_invoke(std::_Any_data const&, JSONRPCRequest const&, UniValue&, bool&&) (__functor=..., __args#0=..., __args#1=..., __args#2=@0x7f5576ffbf44: true) at /usr/include/c++/12/bits/std_function.h:290
#14 0x00005639359302c6 in std::function<bool (JSONRPCRequest const&, UniValue&, bool)>::operator()(JSONRPCRequest const&, UniValue&, bool) const (this=0x5639368fbb00 <RegisterOutputScriptRPCCommands(CRPCTable&)::commands+320>,
    __args#0=..., __args#1=..., __args#2=true) at /usr/include/c++/12/bits/std_function.h:591
#15 0x0000563935b0afd5 in ExecuteCommand (command=..., request=..., result=..., last_handler=true) at rpc/server.cpp:475
#16 0x0000563935b0abae in ExecuteCommands (commands=std::vector of length 1, capacity 1 = {...}, request=..., result=...) at rpc/server.cpp:440
#17 0x0000563935b0ad52 in CRPCTable::execute (this=0x5639368fc4c0 <tableRPC>, request=...) at rpc/server.cpp:460
#18 0x0000563935cbe3f5 in HTTPReq_JSONRPC (context=std::any containing node::NodeContext * = {...}, req=0x7f556c002da0) at httprpc.cpp:201
#19 0x0000563935cc0548 in operator() (__closure=0x7f556c001980, req=0x7f556c002da0) at httprpc.cpp:300
#20 0x0000563935cc1c3a in std::__invoke_impl<bool, StartHTTPRPC(const std::any&)::<lambda(HTTPRequest*, const std::string&)>&, HTTPRequest*, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&>(std::__invoke_other, struct {...} &) (__f=...) at /usr/include/c++/12/bits/invoke.h:61
#21 0x0000563935cc1a78 in std::__invoke_r<bool, StartHTTPRPC(const std::any&)::<lambda(HTTPRequest*, const std::string&)>&, HTTPRequest*, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&>(struct {...} &) (__fn=...) at /usr/include/c++/12/bits/invoke.h:114
#22 0x0000563935cc182e in std::_Function_handler<bool(HTTPRequest*, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&), StartHTTPRPC(const std::any&)::<lambda(HTTPRequest*, const std::string&)> >::_M_invoke(const std::_Any_data &, HTTPRequest *&&, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > &) (__functor=..., __args#0=@0x7f5576ffcb80: 0x7f556c002da0, __args#1="")
    at /usr/include/c++/12/bits/std_function.h:290
#23 0x0000563935cd1c0c in std::function<bool (HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)>::operator()(HTTPRequest*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const (this=0x7f556c003fb0, __args#0=0x7f556c002da0, __args#1="") at /usr/include/c++/12/bits/std_function.h:591
#24 0x0000563935cd0fc2 in HTTPWorkItem::operator() (this=0x7f556c003f80) at httpserver.cpp:56
#25 0x0000563935cd326d in WorkQueue<HTTPClosure>::Run (this=0x563938765d10) at httpserver.cpp:111
#26 0x0000563935ccbf1d in HTTPWorkQueueRun (queue=0x563938765d10, worker_num=3) at httpserver.cpp:343
#27 0x0000563935cdfd51 in std::__invoke_impl<void, void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int> (__f=@0x56393877e7f8: 0x563935ccbeb2 <HTTPWorkQueueRun(WorkQueue<HTTPClosure>*, int)>)
    at /usr/include/c++/12/bits/invoke.h:61
#28 0x0000563935cdfaaa in std::__invoke<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int> (__fn=@0x56393877e7f8: 0x563935ccbeb2 <HTTPWorkQueueRun(WorkQueue<HTTPClosure>*, int)>)
    at /usr/include/c++/12/bits/invoke.h:96
#29 0x0000563935cdf8a6 in std::thread::_Invoker<std::tuple<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int> >::_M_invoke<0ul, 1ul, 2ul> (this=0x56393877e7e8) at /usr/include/c++/12/bits/std_thread.h:252
#30 0x0000563935cdf7db in std::thread::_Invoker<std::tuple<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int> >::operator() (this=0x56393877e7e8) at /usr/include/c++/12/bits/std_thread.h:259
#31 0x0000563935cdf60f in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(WorkQueue<HTTPClosure>*, int), WorkQueue<HTTPClosure>*, int> > >::_M_run (this=0x56393877e7e0) at /usr/include/c++/12/bits/std_thread.h:210
#32 0x00007f55a8e15b03 in execute_native_thread_routine () from /lib64/libstdc++.so.6
#33 0x00007f55a8ac4e2d in start_thread () from /lib64/libc.so.6
#34 0x00007f55a8b4a1b0 in clone3 () from /lib64/libc.so.6
```

The affected code seems to be in frame 4, and corresponds to the following code:

```
(gdb) frame 4
#4  0x0000563935a84d73 in operator() (__closure=0x7f5576ffbd50, self=..., request=...) at rpc/output_script.cpp:276
276                 for (int i = range_begin; i <= range_end; ++i) {
```

I think the reason is that, while `range_begin` and `range_end` are `uint64_t`, `i` is an `int`, which on many platforms means `int32_t`.
When `i` is assigned `2^31-1` and is then incremented, it wraps back and causes the crash.



Discussion / hints:

Fix patch:
diff --git a/src/rpc/output_script.cpp b/src/rpc/output_script.cpp
index 744f8098147c7..a980c609e808a 100644
--- a/src/rpc/output_script.cpp
+++ b/src/rpc/output_script.cpp
@@ -273,7 +273,7 @@ static RPCHelpMan deriveaddresses()
 
             UniValue addresses(UniValue::VARR);
 
-            for (int i = range_begin; i <= range_end; ++i) {
+            for (int64_t i = range_begin; i <= range_end; ++i) {
                 FlatSigningProvider provider;
                 std::vector<CScript> scripts;
                 if (!desc->Expand(i, key_provider, scripts, provider)) {


Test patch:
diff --git a/test/functional/rpc_deriveaddresses.py b/test/functional/rpc_deriveaddresses.py
index 42d7d59d56313..a69326736d73c 100755
--- a/test/functional/rpc_deriveaddresses.py
+++ b/test/functional/rpc_deriveaddresses.py
@@ -44,6 +44,13 @@ def run_test(self):
         combo_descriptor = descsum_create("combo(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/0)")
         assert_equal(self.nodes[0].deriveaddresses(combo_descriptor), ["mtfUoUax9L4tzXARpw1oTGxWyoogp52KhJ", "mtfUoUax9L4tzXARpw1oTGxWyoogp52KhJ", address, "2NDvEwGfpEqJWfybzpKPHF2XH3jwoQV3D7x"])
 
+        # Before #26275, bitcoind would crash when deriveaddresses was
+        # called with derivation index 2147483647, which is the maximum
+        # positive value of a signed int32, and - currently - the
+        # maximum value that the deriveaddresses bitcoin RPC call
+        # accepts as derivation index.
+        assert_equal(self.nodes[0].deriveaddresses(descsum_create("wpkh(tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/1/*)"), [2147483647, 2147483647]), ["bcrt1qtzs23vgzpreks5gtygwxf8tv5rldxvvsyfpdkg"])
+
         hardened_without_privkey_descriptor = descsum_create("wpkh(tpubD6NzVbkrYhZ4WaWSyoBvQwbpLkojyoTZPRsgXELWz3Popb3qkjcJyJUGLnL4qHHoQvao8ESaAstxYSnhyswJ76uZPStJRJCTKvosUCJZL5B/1'/1/0)")
         assert_raises_rpc_error(-5, "Cannot derive script without private keys", self.nodes[0].deriveaddresses, hardened_without_privkey_descriptor)