Skip to content

Commit 349ed2a

Browse files
committed
wallet: throw error if legacy entries are present on loading descriptor wallets
In the wallet key-value-loading routine, most legacy type entries require a LegacyScriptPubKeyMan instance after successful deserialization. On a descriptor wallet, creating that (via method `GetOrCreateLegacyScriptPubKeyMan`) fails and then leads to a null-pointer dereference crash. Fix this by throwing an error if if the wallet flags indicate that we have a descriptor wallet and there is a legacy entry found.
1 parent 50422b7 commit 349ed2a

File tree

3 files changed

+18
-1
lines changed

3 files changed

+18
-1
lines changed

src/wallet/wallet.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2919,6 +2919,10 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
29192919
"The wallet might had been created on a newer version.\n"
29202920
"Please try running the latest software version.\n"), walletFile);
29212921
return nullptr;
2922+
} else if (nLoadWalletRet == DBErrors::UNEXPECTED_LEGACY_ENTRY) {
2923+
error = strprintf(_("Unexpected legacy entry in descriptor wallet found. Loading wallet %s\n\n"
2924+
"The wallet might have been tampered with or created with malicious intent.\n"), walletFile);
2925+
return nullptr;
29222926
} else {
29232927
error = strprintf(_("Error loading %s"), walletFile);
29242928
return nullptr;

src/wallet/walletdb.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,7 @@ class CWalletScanState {
315315
std::map<uint160, CHDChain> m_hd_chains;
316316
bool tx_corrupt{false};
317317
bool descriptor_unknown{false};
318+
bool unexpected_legacy_entry{false};
318319

319320
CWalletScanState() = default;
320321
};
@@ -332,6 +333,11 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
332333
if (filter_fn && !filter_fn(strType)) {
333334
return true;
334335
}
336+
// Legacy entries in descriptor wallets are not allowed, abort immediately
337+
if (pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS) && DBKeys::LEGACY_TYPES.count(strType) > 0) {
338+
wss.unexpected_legacy_entry = true;
339+
return false;
340+
}
335341
if (strType == DBKeys::NAME) {
336342
std::string strAddress;
337343
ssKey >> strAddress;
@@ -833,6 +839,12 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
833839
std::string strType, strErr;
834840
if (!ReadKeyValue(pwallet, ssKey, ssValue, wss, strType, strErr))
835841
{
842+
if (wss.unexpected_legacy_entry) {
843+
strErr = strprintf("Error: Unexpected legacy entry found in descriptor wallet %s. ", pwallet->GetName());
844+
strErr += "The wallet might have been tampered with or created with malicious intent.";
845+
pwallet->WalletLogPrintf("%s\n", strErr);
846+
return DBErrors::UNEXPECTED_LEGACY_ENTRY;
847+
}
836848
// losing keys is considered a catastrophic error, anything else
837849
// we assume the user can live with:
838850
if (IsKeyType(strType) || strType == DBKeys::DEFAULTKEY) {

src/wallet/walletdb.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ enum class DBErrors
5252
LOAD_FAIL,
5353
NEED_REWRITE,
5454
NEED_RESCAN,
55-
UNKNOWN_DESCRIPTOR
55+
UNKNOWN_DESCRIPTOR,
56+
UNEXPECTED_LEGACY_ENTRY
5657
};
5758

5859
namespace DBKeys {

0 commit comments

Comments
 (0)