-
Notifications
You must be signed in to change notification settings - Fork 37.7k
wallet: fix segfault by avoiding invalid default-ctored external_spk_managers
entry
#23333
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.
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
wallet: fix segfault by avoiding invalid default-ctored external_spk_managers
entry
#23333
Conversation
…_managers` entry In the method `CWallet::LoadActiveScriptPubKeyMan`, the map `external_spk_managers` (or `internal_spk_managers`, if parameter `internal` is false) is accessed via std::map::operator[], which means that a default-ctored entry is created with a null-pointer as value, if the key doesn't exist. As soon as this value is dereferenced, a segmentation fault occurs, e.g. in `CWallet::KeypoolCountExternalKeys`. The bevaviour can be reproduced by the following steps (starting with empty regtest datadir): $ ./src/bitcoind -regtest -daemon $ ./src/bitcoin-cli -regtest -named createwallet_name=wallet descriptors=true blank=true $ cat regtest-descriptors.txt [ { "desc": "tr([e4445899/49'/1'/0']tprv8ZgxMBicQKsPd8jCeBWsYLEoWxbVgzJDatJ7XkwQ6G3uF4FsHuaziHQ5JZAW4K515nj6kVVwPaNWZSMEcR7aFCwL4tQqTcaoprMKTTtm6Zg/1/*)#mr3llm7f", "timestamp": 1634652324, "active": true, "internal": true, "range": [ 0, 999 ], "next": 0 } ] $ ./src/bitcoin-cli -regtest importdescriptors "$(cat regtest-descriptors.txt)" [ { "success": true } ] $ ./src/bitcoin-cli -regtest getwalletinfo error: timeout on transient error: Could not connect to the server 127.0.0.1:18443 (error code 1 - "EOF reached") Bug reported by Josef Vondrlik (josef-v).
Code Review ACK 6911ab9 |
Does this need backport to earlier than 22.x? |
No, this is new in 22.x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested ACK 6911ab9 on Ubuntu 20.04.
I was able to reproduce the error on the master branch and this PR fixes this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 6911ab9
…_managers` entry In the method `CWallet::LoadActiveScriptPubKeyMan`, the map `external_spk_managers` (or `internal_spk_managers`, if parameter `internal` is false) is accessed via std::map::operator[], which means that a default-ctored entry is created with a null-pointer as value, if the key doesn't exist. As soon as this value is dereferenced, a segmentation fault occurs, e.g. in `CWallet::KeypoolCountExternalKeys`. The bevaviour can be reproduced by the following steps (starting with empty regtest datadir): $ ./src/bitcoind -regtest -daemon $ ./src/bitcoin-cli -regtest -named createwallet_name=wallet descriptors=true blank=true $ cat regtest-descriptors.txt [ { "desc": "tr([e4445899/49'/1'/0']tprv8ZgxMBicQKsPd8jCeBWsYLEoWxbVgzJDatJ7XkwQ6G3uF4FsHuaziHQ5JZAW4K515nj6kVVwPaNWZSMEcR7aFCwL4tQqTcaoprMKTTtm6Zg/1/*)#mr3llm7f", "timestamp": 1634652324, "active": true, "internal": true, "range": [ 0, 999 ], "next": 0 } ] $ ./src/bitcoin-cli -regtest importdescriptors "$(cat regtest-descriptors.txt)" [ { "success": true } ] $ ./src/bitcoin-cli -regtest getwalletinfo error: timeout on transient error: Could not connect to the server 127.0.0.1:18443 (error code 1 - "EOF reached") Bug reported by Josef Vondrlik (josef-v). Github-Pull: bitcoin#23333 Rebased-From: 6911ab9
Added to #23276 for backporting to 22.x |
…-ctored `external_spk_managers` entry 6911ab9 wallet: fix segfault by avoiding invalid default-ctored `external_spk_managers` entry (Sebastian Falbesoner) Pull request description: Fixes bitcoin#23321 (bug reported by Josef Vondrlik (josef-v)). In the method `CWallet::LoadActiveScriptPubKeyMan`, the map `external_spk_managers` (or `internal_spk_managers`, if parameter `internal` is false) is accessed via std::map::operator[], which means that a default-ctored entry is created with a null-pointer as value, if the key doesn't exist. As soon as this value is dereferenced, a segmentation fault occurs, e.g. in `CWallet::KeypoolCountExternalKeys`. The bevaviour can be reproduced by the following steps (starting with empty regtest datadir): ``` $ ./src/bitcoind -regtest -daemon $ ./src/bitcoin-cli -regtest -named createwallet_name=wallet descriptors=true blank=true $ cat regtest-descriptors.txt [ { "desc": "tr([e4445899/49'/1'/0']tprv8ZgxMBicQKsPd8jCeBWsYLEoWxbVgzJDatJ7XkwQ6G3uF4FsHuaziHQ5JZAW4K515nj6kVVwPaNWZSMEcR7aFCwL4tQqTcaoprMKTTtm6Zg/1/*)#mr3llm7f", "timestamp": 1634652324, "active": true, "internal": true, "range": [ 0, 999 ], "next": 0 } ] $ ./src/bitcoin-cli -regtest importdescriptors "$(cat regtest-descriptors.txt)" [ { "success": true } ] $ ./src/bitcoin-cli -regtest getwalletinfo error: timeout on transient error: Could not connect to the server 127.0.0.1:18443 (error code 1 - "EOF reached") ``` ACKs for top commit: achow101: Code Review ACK 6911ab9 lsilva01: Tested ACK 6911ab9 on Ubuntu 20.04. instagibbs: ACK 6911ab9 Tree-SHA512: 76aa96847cf2739413fb68fb902afef0b3ab9381178dd62fb0abac69f853f1f6523d73c60e610375b9a7730f275eda9162503b89f5be6e6e349a8d047b59c8dc
…_managers` entry In the method `CWallet::LoadActiveScriptPubKeyMan`, the map `external_spk_managers` (or `internal_spk_managers`, if parameter `internal` is false) is accessed via std::map::operator[], which means that a default-ctored entry is created with a null-pointer as value, if the key doesn't exist. As soon as this value is dereferenced, a segmentation fault occurs, e.g. in `CWallet::KeypoolCountExternalKeys`. The bevaviour can be reproduced by the following steps (starting with empty regtest datadir): $ ./src/bitcoind -regtest -daemon $ ./src/bitcoin-cli -regtest -named createwallet_name=wallet descriptors=true blank=true $ cat regtest-descriptors.txt [ { "desc": "tr([e4445899/49'/1'/0']tprv8ZgxMBicQKsPd8jCeBWsYLEoWxbVgzJDatJ7XkwQ6G3uF4FsHuaziHQ5JZAW4K515nj6kVVwPaNWZSMEcR7aFCwL4tQqTcaoprMKTTtm6Zg/1/*)#mr3llm7f", "timestamp": 1634652324, "active": true, "internal": true, "range": [ 0, 999 ], "next": 0 } ] $ ./src/bitcoin-cli -regtest importdescriptors "$(cat regtest-descriptors.txt)" [ { "success": true } ] $ ./src/bitcoin-cli -regtest getwalletinfo error: timeout on transient error: Could not connect to the server 127.0.0.1:18443 (error code 1 - "EOF reached") Bug reported by Josef Vondrlik (josef-v). Github-Pull: bitcoin#23333 Rebased-From: 6911ab9
…_managers` entry In the method `CWallet::LoadActiveScriptPubKeyMan`, the map `external_spk_managers` (or `internal_spk_managers`, if parameter `internal` is false) is accessed via std::map::operator[], which means that a default-ctored entry is created with a null-pointer as value, if the key doesn't exist. As soon as this value is dereferenced, a segmentation fault occurs, e.g. in `CWallet::KeypoolCountExternalKeys`. The bevaviour can be reproduced by the following steps (starting with empty regtest datadir): $ ./src/bitcoind -regtest -daemon $ ./src/bitcoin-cli -regtest -named createwallet_name=wallet descriptors=true blank=true $ cat regtest-descriptors.txt [ { "desc": "tr([e4445899/49'/1'/0']tprv8ZgxMBicQKsPd8jCeBWsYLEoWxbVgzJDatJ7XkwQ6G3uF4FsHuaziHQ5JZAW4K515nj6kVVwPaNWZSMEcR7aFCwL4tQqTcaoprMKTTtm6Zg/1/*)#mr3llm7f", "timestamp": 1634652324, "active": true, "internal": true, "range": [ 0, 999 ], "next": 0 } ] $ ./src/bitcoin-cli -regtest importdescriptors "$(cat regtest-descriptors.txt)" [ { "success": true } ] $ ./src/bitcoin-cli -regtest getwalletinfo error: timeout on transient error: Could not connect to the server 127.0.0.1:18443 (error code 1 - "EOF reached") Bug reported by Josef Vondrlik (josef-v). Github-Pull: bitcoin#23333 Rebased-From: 6911ab9
269553f test: Call ceildiv helper with integer (Martin Zumsande) 2f60fc6 ci: Replace soon EOL hirsute with jammy (MarcoFalke) 801b0f0 build: patch qt to explicitly define previously implicit header include (Kittywhiskers Van Gogh) c768bfa tests: Calculate fees more similarly to CFeeRate::GetFee (Andrew Chow) f66bc42 tests: Test for assertion when feerate is rounded down (Andrew Chow) bd7e08e fees: Always round up fee calculated from a feerate (Andrew Chow) 227ae65 wallet: fix segfault by avoiding invalid default-ctored `external_spk_managers` entry (Sebastian Falbesoner) 282863a refactor: include a missing <limits> header in fs.cpp (Joan Karadimov) 7febe4f consensus: don't call GetBlockPos in ReadBlockFromDisk without lock (Jon Atack) c671c6f the result of CWallet::IsHDEnabled() was initialized with true. (Saibato) a5a1538 build, qt: Fix typo in QtInputSupport check (Hennadii Stepanov) c95b188 system: skip trying to set the locale on NetBSD (fanquake) c1cdedd guix: Fix powerpc64(le) dynamic linker name (Carl Dong) 92d44ff doc: Add 23061 release notes (MarcoFalke) db76db7 Fix (inverse) meaning of -persistmempool (MarcoFalke) 85c78e0 build: Restrict check for CRC32C intrinsic to aarch64 (W. J. van der Laan) Pull request description: Collecting backports for the 22.1 release. Currently: * #23045 * #23061 * #23148 * #22390 * #22820 * #22781 * #22895 * #23335 * #23333 * #22949 * #23580 * #23504 * #24239 ACKs for top commit: achow101: ACK 269553f Tree-SHA512: b3a57ea241be7a83488eeb032276f4cf82a0987aada906a82f94a20c4acf9f2397708249dcecbe1c7575e70d09c60b835233d4718af4013c7bc58896c618274c
Fixes #23321 (bug reported by Josef Vondrlik (josef-v)).
In the method
CWallet::LoadActiveScriptPubKeyMan
, the mapexternal_spk_managers
(orinternal_spk_managers
, if parameterinternal
is false) is accessed via std::map::operator[], which means that a default-ctored entry is created with a null-pointer as value, if the key doesn't exist. As soon as this value is dereferenced, a segmentation fault occurs, e.g. inCWallet::KeypoolCountExternalKeys
.The bevaviour can be reproduced by the following steps (starting with empty regtest datadir):