diff options
author | fanquake <fanquake@gmail.com> | 2022-12-01 10:16:58 +0000 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2022-12-01 10:17:09 +0000 |
commit | e334f7a54592ba9f05e4a5578dd933a7a31c3444 (patch) | |
tree | 146e0828fcd7a61b78ab7da908ba43b43159e868 /src/wallet/wallet.cpp | |
parent | e2bfd41f832dc7c7be6f17e928352f0eb2865f66 (diff) | |
parent | 5e65a216d1fd00c447757736d4f2899d235e731a (diff) | |
download | bitcoin-e334f7a54592ba9f05e4a5578dd933a7a31c3444.tar.xz |
Merge bitcoin/bitcoin#26594: wallet: Avoid a segfault in migratewallet failure cleanup
5e65a216d1fd00c447757736d4f2899d235e731a wallet: Explicitly say migratewallet on encrypted wallets is unsupported (Andrew Chow)
88afc73ae0c67a4482ecd3d77eb2a8fd2673f82d tests: Test for migrating encrypted wallets (Andrew Chow)
86ef7b3c7be84e4183098f448c77ecc9ea7367ab wallet: Avoid null pointer deref when cleaning up migratewallet (Andrew Chow)
Pull request description:
When `migratewallet` fails, we do an automatic cleanup in order to reset everything so that the user does not experience any interruptions. However, this apparently has a segfault in it, caused by the the pointers to the watchonly and solvables wallets being nullptr. If those wallets are not created (either not needed, or failed early on), we will accidentally attempt to dereference these nullptrs, which causes a segfault.
This failure can be easily reached by trying to migrate an encrypted wallet. Currently, we can't migrate encrypted wallets because of how we unload wallets before migrating, and therefore forget the encryption key if the wallet was unlocked. So any encrypted wallets will fail, entering the cleanup, and because watchonly and solvables wallets don't exist yet, the segfault is reached.
This PR fixes this by not putting those nullptrs in a place that we will end up dereferencing them later. It also adds a test that uses the encrypted wallet issue.
ACKs for top commit:
S3RK:
reACK 5e65a216d1fd00c447757736d4f2899d235e731a
stickies-v:
ACK [5e65a21](https://github.com/bitcoin/bitcoin/commit/5e65a216d1fd00c447757736d4f2899d235e731a)
furszy:
diff ACK 5e65a21
Tree-SHA512: f75643797220d4232ad3ab8cb4b46d0f3667f00486e910ca748c9b6d174d446968f1ec4dd7f907da1be9566088849da7edcd8cd8f12de671c3241b513deb8e80
Diffstat (limited to 'src/wallet/wallet.cpp')
-rw-r--r-- | src/wallet/wallet.cpp | 4 |
1 files changed, 2 insertions, 2 deletions
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 431e970edc..0f297b44ab 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4182,8 +4182,8 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet> // Make list of wallets to cleanup std::vector<std::shared_ptr<CWallet>> created_wallets; - created_wallets.push_back(std::move(res.watchonly_wallet)); - created_wallets.push_back(std::move(res.solvables_wallet)); + if (res.watchonly_wallet) created_wallets.push_back(std::move(res.watchonly_wallet)); + if (res.solvables_wallet) created_wallets.push_back(std::move(res.solvables_wallet)); // Get the directories to remove after unloading for (std::shared_ptr<CWallet>& w : created_wallets) { |