tests: wizard: imported addrs: validate each addr with "is_address()"
What the developer wrote
Authored by SomberNight
tests: wizard: imported addrs: validate each addr with "is_address()"
This was already done *in the GUIs*, but the backend should definitely
do at least sanity-check-level validation like this.
What changed, and why it matters
This commit adds a safety check in Electrum's wallet creation wizard to verify that strings entered as 'imported addresses' actually look like valid Bitcoin addresses. Previously, the GUI did this check, but the backend code did not. The change prevents invalid or accidental garbage strings from being accepted as wallet addresses. It is a hardening improvement rather than a fix for a known active attack.
Treat as a routine hardening patch. Review whether assertion-based failure handling is appropriate for production (asserts may be stripped or crash the wizard) and consider replacing with a proper validation exception. Address normalization TODO should be tracked separately.
Security signals we found
Input validation added to backend address import path
Assertion-based sanity check on user-supplied address strings
Test added for invalid address rejection
Commit message explicitly frames change as defensive validation
Evidence from the diff
In electrum/wizard.py, NewWalletWizard now calls bitcoin.is_address(addr) and asserts the result when processing an ‘address_list’ for imported wallets. It also asserts the address is a string. A new unit test confirms that an invalid address string triggers an AssertionError with the expected message. The commit notes that address normalization (e.g., bech32 case) is still not performed and is left as a TODO.
Changed components
electrum/wizard.pytests/test_wizard.pyInspect captured patch +22 / −1
diff --git a/electrum/wizard.py b/electrum/wizard.py
index 48a3cee..6894549 100644
--- a/electrum/wizard.py
+++ b/electrum/wizard.py
@@ -679,6 +679,11 @@ class NewWalletWizard(KeystoreWizard):
addresses[addr] = {'type': txin_type, 'pubkey': pubkey}
elif 'address_list' in data:
for addr in data['address_list'].split():
+ assert isinstance(addr, str)
+ assert bitcoin.is_address(addr), f"expected bitcoin addr. got {addr[:5] + '..' + addr[-2:]}"
+ # note: we do not normalize addresses. :/
+ # In particular, bech32 addresses can be either all-lowercase or all-uppercase.
+ # TODO we should normalize them, but it only makes sense if we also do a walletDB-upgrade.
addresses[addr] = {}
elif data['keystore_type'] in ['createseed', 'haveseed']:
seed_extension = data['seed_extra_words'] if data['seed_extend'] else ''
diff --git a/tests/test_wizard.py b/tests/test_wizard.py
index 95ba511..a494245 100644
--- a/tests/test_wizard.py
+++ b/tests/test_wizard.py
@@ -789,11 +789,27 @@ class WalletWizardTestCase(WizardTestCase):
{
"14gcRovpkCoGkCNBivQBvw7eso7eiNAbxG",
"35ZqQJcBQMZ1rsv8aSuJ2wkC7ohUCQMJbT",
- "BC1QW508D6QEJXTDG4Y5R3ZARVARY0C5XW7KV8F3T4",
+ "BC1QW508D6QEJXTDG4Y5R3ZARVARY0C5XW7KV8F3T4", # TODO normalize to lowercase?
"bc1pw508d6qejxtdg4y5r3zarvary0c5xw7kw508d6qejxtdg4y5r3zarvary0c5xw7kt5nd6y",
},
)
+ async def test_create_imported_wallet_from_addresses__invalid_input(self):
+ w = self._wizard_for(wallet_type='imported')
+ v = w._current
+ d = v.wizard_data
+ self.assertEqual('imported', v.view)
+
+ d.update({
+ 'address_list':
+ 'garbagegarbage\n'
+ '35ZqQJcBQMZ1rsv8aSuJ2wkC7ohUCQMJbT\n'
+ })
+ v = w.resolve_next(v.view, d)
+ with self.assertRaises(AssertionError) as ctx:
+ wallet = self._set_password_and_check_address(v=v, w=w, recv_addr=None)
+ self.assertTrue("expected bitcoin addr" in ctx.exception.args[0])
+
async def test_create_imported_wallet_from_wif_keys(self):
w = self._wizard_for(wallet_type='imported')
v = w._current
Why this scored 42/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.