wallet: fix segfault by avoiding invalid default-ctored external_spk_managers entry #23333

1. 
   theStack commented at 2:31 pm on October 21, 2021: member

   Fixes #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):

    0$ ./src/bitcoind -regtest -daemon
    1$ ./src/bitcoin-cli -regtest -named createwallet_name=wallet descriptors=true blank=true
    2$ cat regtest-descriptors.txt
    3[
    4  {
    5    "desc": "tr([e4445899/49'/1'/0']tprv8ZgxMBicQKsPd8jCeBWsYLEoWxbVgzJDatJ7XkwQ6G3uF4FsHuaziHQ5JZAW4K515nj6kVVwPaNWZSMEcR7aFCwL4tQqTcaoprMKTTtm6Zg/1/*)#mr3llm7f",
    6    "timestamp": 1634652324,
    7    "active": true,
    8    "internal": true,
    9    "range": [
   10      0,
   11      999
   12    ],
   13    "next": 0
   14  }
   15]
   16$ ./src/bitcoin-cli -regtest importdescriptors "$(cat regtest-descriptors.txt)"
   17[
   18  {
   19    "success": true
   20  }
   21]
   22$ ./src/bitcoin-cli -regtest getwalletinfo
   23error: timeout on transient error: Could not connect to the server 127.0.0.1:18443 (error code 1 - "EOF reached")
   

2. wallet: fix segfault by avoiding invalid default-ctored `external_spk_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).

   6911ab95f1
3. laanwj added the label Wallet on Oct 21, 2021
4. 
   achow101 commented at 5:35 pm on October 21, 2021: member

   Code Review ACK 6911ab95f19d2b1f60f2d0b2f3961fa6639d4f31
5. MarcoFalke added the label Needs backport (22.x) on Oct 21, 2021
6. 
   MarcoFalke commented at 5:51 pm on October 21, 2021: member

   Does this need backport to earlier than 22.x?
7. 
   achow101 commented at 5:52 pm on October 21, 2021: member

   Does this need backport to earlier than 22.x?

   No, this is new in 22.x

8. lsilva01 approved
9. 
   lsilva01 commented at 7:23 pm on October 21, 2021: contributor

   Tested ACK 6911ab9 on Ubuntu 20.04. I was able to reproduce the error on the master branch and this PR fixes this.
10. fanquake requested review from instagibbs on Oct 22, 2021
11. instagibbs approved
12. 
    instagibbs commented at 11:12 am on October 22, 2021: member

    ACK 6911ab95f19d2b1f60f2d0b2f3961fa6639d4f31
13. fanquake merged this on Oct 22, 2021
14. fanquake closed this on Oct 22, 2021

    ---

15. fanquake referenced this in commit 99a2470b74 on Oct 22, 2021
16. 
    fanquake commented at 12:37 pm on October 22, 2021: member

    Added to #23276 for backporting to 22.x
17. fanquake removed the label Needs backport (22.x) on Oct 22, 2021
18. sidhujag referenced this in commit 10c541073f on Oct 22, 2021
19. fanquake referenced this in commit 54b6ca3dbe on Feb 14, 2022
20. fanquake referenced this in commit 227ae65254 on Feb 15, 2022
21. fanquake referenced this in commit 9b5f674abb on Mar 1, 2022
22. DrahtBot locked this on Oct 30, 2022

Contributors
theStack achow101 MarcoFalke lsilva01 instagibbs fanquake

Labels
Wallet