Seed phrase saved in plaintext even with a wallet password (v0.2.0-beta.1)

[rui-sv]

Hi! I asked ChatGPT to check how ElectrumSVP v0.2.0-beta.1 generates and stores seed phrases, and it found something that looks concerning.

When creating a new standard account, the wallet appears to save a plaintext copy of the BIP39 seed phrase in its database, even with a wallet password set. Changing the password also seems to leave that copy untouched.

Here’s the code path it found, based on commit:

4a09e01

1. In electrumsv/gui/qt/account_wizard.py, around line 429, the account creation code does this:
   keystore.bip39_words = seed_phrase
   This keeps an extra copy of the original seed phrase in bip39_words.
2. In electrumsv/keystore.py, around line 485, the password-update code encrypts fields such as seed and xprv, but doesn’t handle bip39_words.
3. In electrumsv/keystore.py, around line 449, the serialization code includes bip39_words in the data saved to the wallet database. That means this copy gets saved without encryption.

The review tested this at the database level using test data with no funds. It was able to read the full seed phrase without the wallet password and use it to recreate the same account extended private key. After changing the password and saving again, the plaintext seed phrase was still there.

So if someone gets access to an affected wallet file or a backup, they could recover the seed phrase despite the wallet having a password. In the tested setup, that was enough to recover the account’s private key.

The seed-generation checks themselves passed: the code uses os.urandom(16) for 128 bits of entropy, the English wordlist matched BIP39, and all 16 mnemonic-generation tests and 24 seed-derivation tests passed.

The review also inspected the relevant code inside the standard Windows EXE and found that it matched the tagged source. The reproduction used the wallet/database code directly, so it wasn’t a full test through the packaged app’s interface. The portable Windows, macOS, and Linux packages weren’t separately checked.

Could you take a look and confirm this? It seems the fix would need to cover both newly created accounts and existing wallet files that already contain a plaintext bip39_words field.

It would also be helpful to let affected users know what to do about their wallets and backups, since changing the password alone doesn’t fix this, and any seed phrase that has already been exposed would still be compromised.

Thanks!

[TruthMachine]

Thanks for the detailed report and reproduction. You correctly identified a problem in the wallet serialization path.

I have confirmed that affected versions could retain the BIP39 mnemonic in the bip39_words field in plaintext. The wallet password protected the encrypted seed/key material, but this additional field was not being protected by the wallet password.

This has been fixed in the current build code.

The fix addresses both newly created and existing wallets:

• Newly created BIP39 accounts no longer retain the mnemonic in the plaintext bip39_words field.
• Existing wallet files containing the legacy plaintext field are migrated when opened.
• The plaintext bip39_words data is removed during migration.
• The encrypted wallet key/seed data remains available so the wallet can continue to be used normally.
• Changing the wallet password no longer leaves this legacy plaintext field behind.

I am currently completing the migration testing and preparing the 0.2.0 release.

For users of affected versions, changing the wallet password alone is not sufficient to remove plaintext data that may already exist in an older wallet file or backup. Once the fix is released, affected wallets should be opened with the fixed version and be allowed to migrate automatically, followed by creation of a fresh backup. Old wallet files and backups containing the plaintext mnemonic should be treated accordingly.

If a mnemonic has actually been exposed to an untrusted party, it should be considered compromised and the funds should be moved to a newly generated wallet.

Thanks again for catching this and for providing such a detailed reproduction. The testing you did also helped confirm that the issue was in the wallet storage/serialization path rather than the BIP39 mnemonic generation itself.

[TruthMachine]

This is now fixed and user wallets are migrated in the new version. Everyone is advised to move to the new version and if wallet backup files were exposed, users should consider those wallets insecure and move to a new seed phrase.

https://github.com/TruthMachine/ElectrumSVP/releases/tag/v0.2.0