timelock_recovery: recovery destination checks for address is_mine or script output
What changed, and why it matters
This commit adds safety checks to a wallet recovery feature. It prevents users from accidentally setting a recovery transaction that sends funds back to the same wallet they are trying to recover, or to a non-address destination. The commit message notes that frontend validation should also exist in backend code, suggesting this is only a partial fix in the user interface layer.
Add equivalent validation to the backend timelock_recovery logic so the checks cannot be bypassed by non-Qt clients or crafted requests. Review whether non-address script outputs should ever be permitted for recovery and document the intended behavior.
Security signals we found
Self-send recovery destination could allow a recovery transaction to be ineffective or circular
Missing address validation could allow recovery to arbitrary script outputs
Commit message indicates frontend-only fix; backend validation still needed
Partial patch increases residual risk
Evidence from the diff
The patch modifies electrum/plugins/timelock_recovery/qt.py to validate recovery transaction destinations in the Qt GUI. For multiline outputs, it now rejects outputs without an address and rejects addresses that belong to the same wallet (is_mine). For single-line payment identifiers, it rejects addresses that are is_mine. The commit message explicitly states that backend validation should also be added, implying the server-side or non-Qt code paths may still lack these checks.
Changed components
electrum/plugins/timelock_recovery/qt.pyTimelock Recovery plugin Qt GUIInspect captured patch +14 / −0
diff --git a/electrum/plugins/timelock_recovery/qt.py b/electrum/plugins/timelock_recovery/qt.py
index 26c38d4..6c0d0a7 100644
--- a/electrum/plugins/timelock_recovery/qt.py
+++ b/electrum/plugins/timelock_recovery/qt.py
@@ -463,6 +463,16 @@ class Plugin(TimelockRecoveryPlugin):
payto_e.setStyleSheet(ColorScheme.RED.as_stylesheet(True))
payto_e.setToolTip("At least one line must be set to max spend ('!' in the amount column).")
return False
+ for output in pi.multiline_outputs: # type: PartialTxOutput
+ if not output.address:
+ payto_e.setStyleSheet(ColorScheme.RED.as_stylesheet(True))
+ payto_e.setToolTip("Recovery should only send to addresses.")
+ return False
+ else:
+ if context.wallet.is_mine(output.address):
+ payto_e.setStyleSheet(ColorScheme.RED.as_stylesheet(True))
+ payto_e.setToolTip("Recovery should not send to same wallet.")
+ return False
context.outputs = pi.multiline_outputs
else:
if not pi.is_available() or pi.type != PaymentIdentifierType.SPK or not pi.spk_is_address:
@@ -470,6 +480,10 @@ class Plugin(TimelockRecoveryPlugin):
payto_e.setToolTip("Invalid address type - must be a Bitcoin address.")
return False
assert pi.spk and pi.spk_is_address
+ if context.wallet.is_mine(pi.text):
+ payto_e.setStyleSheet(ColorScheme.RED.as_stylesheet(True))
+ payto_e.setToolTip("Recovery should not send to same wallet.")
+ return False
context.outputs = [PartialTxOutput(scriptpubkey=pi.spk, value='!')]
return True
Why this scored 46/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.