Fix Crash on Inactive HD Seed Key Top-Up When meta.key_origin.path is Too Short

Summary

This pull request fixes a crash in the DigiByte Core wallet (v8.22.0) that occurrs during rescanning and reindexing of older HD wallets. The issue was identified in the LegacyScriptPubKeyMan::MarkUnusedAddresses method, where the wallet attempted to access elements [1] and [2] of meta.key_origin.path without verifying that the vector contained at least three elements. This oversight resulted in an out-of-range memory access, causing the crash.

Over recent weeks, we isolated a bug affecting a particular wallet with over 5,000 transactions (originally created in v7.17.2/3). Investigation revealed that older wallet metadata did not always include the complete three-component derivation path.

Notably, similar issues were encountered by BTC in their versions v21 and v22 (wallets originally created with v0.17). Although BTC provided a more comprehensive fix in v23 (Bitcoin PR #23304), we are unable to integrate that solution fully until the broader changes planned for v8.23 are implemented.

This bug was reproduced on Windows and Mac v8.22.0 core wallets multiple times.

This temporary fix has been thoroughly tested on our test case wallet and was also previously reviewed by BTC developers (see Bitcoin PR #23277 for context). We plan to upgrade to v8.23 as soon as possible after taproot activation for more robust HD wallet handling.

The Crash Issue

Debug Backtrace

Debug Backtrace

Below is the relevant backtrace excerpt observed in LLDB:

* thread #60, name = 'b-qt-walletctrl', stop reason = EXC_BAD_ACCESS (code=1, address=0x4)
  * frame #0: 0x0000000100e9d334 digibyte-qt`LegacyScriptPubKeyMan::MarkUnusedAddresses(this=0x0000000521679c50, script=0x00000005231f4708) at scriptpubkeyman.cpp:378:34
   375          if (it != mapKeyMetadata.end()){
   376              CKeyMetadata meta = it->second;
   377              if (!meta.hd_seed_id.IsNull() && meta.hd_seed_id != m_hd_chain.seed_id) {
-> 378                  bool internal = (meta.key_origin.path[1] & ~BIP32_HARDENED_KEY_LIMIT) != 0;
   379                  int64_t index = meta.key_origin.path[2] & ~BIP32_HARDENED_KEY_LIMIT;
   380 
   381                  if (!TopUpInactiveHDChain(meta.hd_seed_id, index, internal)) {

In this scenario, the code assumed that meta.key_origin.path contained at least three elements, which was not the case for older HD wallets.

Why It Was Bad

Out-of-Range Access: When meta.key_origin.path.size() < 3, accessing elements [1] and [2] resulted in reading beyond allocated memory.

Legacy Metadata Vulnerability: Wallets created with older DigiByte versions or those partially upgraded had incomplete derivation paths, leading to crashes during rescanning or reindexing.

How We Fixed It

The fix adds a conditional check to verify that meta.key_origin.path contains at least three elements before accessing indices [1] and [2]. If the path is too short, the code logs a warning and skips the call to TopUpInactiveHDChain, thereby avoiding the crash.

Code Diff

Original Code:

Testing & Future Considerations

Rescanning or Reindexing: We have verified that the crash no longer occurs on older wallets with incomplete derivation paths. Instead, a warning is logged when an insufficient path size is detected.

Future Enhancements: In the future, it may be beneficial to convert or augment legacy seed metadata to ensure all necessary path components are present. For now, this patch prevents a crash by failing gracefully.

All 470 Unit Tests Pass:

src/test/test_digibyte --show_progress

All 215 Functional Tests Pass:

test/functional/test_runner.py --jobs=4

Acknowledgements

Thank you to everyone who assisted in debugging this issue and testing the fix, especially Brian. Your contributions have been essential in resolving this critical crash bug.

Please review and merge this pull request. We plan to integrate a more comprehensive upgrade in v8.23 following taproot activation to further stabilize HD wallets.