Merge pull request #10791 from SomberNight/202608_wallet_sign_message2
What changed, and why it matters
This change moves the trimming of leading/trailing spaces in message-signing fields from the user-interface code into the shared wallet code. For the graphical interfaces (Qt and QML), spaces are still stripped automatically for convenience. For the command-line interface, a new option disables that stripping so that exact user input is preserved. It is a usability/behavior consistency fix, not a fix for an active security vulnerability.
No immediate security action required. Users relying on CLI message signing/verification should be aware that whitespace is now preserved, so signatures produced via CLI for messages with leading/trailing whitespace will differ from GUI-produced signatures for the same visually displayed text. Developers should ensure documentation reflects this behavior difference.
Security signals we found
Behavior change in message signing/verification input handling
CLI now preserves whitespace, which could affect signature validity for messages that intentionally contain leading or trailing whitespace
No memory-safety, cryptographic, or authentication flaw visible in the diff
Evidence from the diff
The commit refactors input whitespace handling for Bitcoin message signing and verification. It introduces a strip_inputs parameter (defaulting to True) on Abstract_Wallet.sign_message and Abstract_Wallet.verify_message. GUI callers (Qt, QML) no longer strip inputs themselves and rely on the default strip_inputs=True. CLI callers (commands.py) explicitly pass strip_inputs=False so that whitespace is preserved for command-line users. The change is described by the author as improving UX, not as a security fix.
Changed components
electrum/wallet.pyelectrum/commands.pyelectrum/gui/qt/main_window.pyelectrum/gui/qml/qewallet.pyelectrum/gui/qml/qedaemon.pyInspect captured patch +27 / −18
### electrum/commands.py
@@ -918,7 +918,12 @@ async def signmessage(self, address, message, password=None, wallet: Abstract_Wa
raise UserFacingException(f"address must be a str instead of {type(address)}")
if not isinstance(message, str):
raise UserFacingException(f"message must be a str instead of {type(message)}")
- sig = wallet.sign_message(address=address, message=message, password=password)
+ sig = wallet.sign_message(
+ address=address,
+ message=message,
+ password=password,
+ strip_inputs=False, # respect whitespaces for CLI
+ )
return base64.b64encode(sig).decode('ascii')
@command('')
@@ -935,7 +940,12 @@ async def verifymessage(self, address, signature, message):
raise UserFacingException(f"signature must be a str instead of {type(signature)}")
if not isinstance(message, str):
raise UserFacingException(f"message must be a str instead of {type(message)}")
- return Abstract_Wallet.verify_message(address=address, signature=signature, message=message)
+ return Abstract_Wallet.verify_message(
+ address=address,
+ signature=signature,
+ message=message,
+ strip_inputs=False, # respect whitespaces for CLI
+ )
def _get_fee_policy(self, fee: str, feerate: str):
if fee is not None and feerate is not None:
### electrum/gui/qml/qedaemon.py
@@ -506,9 +506,6 @@ def startNetwork(self):
@pyqtSlot(str, str, str, result=bool)
def verifyMessage(self, address, message, signature):
- address = address.strip()
- message = message.strip()
- signature = signature.strip()
try:
return Abstract_Wallet.verify_message(address=address, signature=signature, message=message)
except UserFacingException as e:
### electrum/gui/qml/qewallet.py
@@ -848,9 +848,6 @@ def isAddressMine(self, addr):
@pyqtSlot(str, str)
@auth_protect(message=_("Sign message?"))
def signMessage(self, address, message):
- # strip, as in qt gui and in qml verifyMessage (see #4327)
- address = address.strip()
- message = message.strip()
try:
sig = self.wallet.sign_message(address=address, message=message, password=self.password)
except UserFacingException as e:
### electrum/gui/qt/main_window.py
@@ -2132,12 +2132,10 @@ def do_sign(
signature_e: ButtonsTextEdit,
password,
) -> None:
- address = address_e.text().strip()
- message = message_e.toPlainText().strip()
task = partial(
self.wallet.sign_message,
- address=address,
- message=message,
+ address=address_e.text(),
+ message=message_e.toPlainText(),
password=password,
)
@@ -2157,13 +2155,11 @@ def do_verify(
message_e: QTextEdit,
signature_e: ButtonsTextEdit,
) -> None:
- address = address_e.text().strip()
- message = message_e.toPlainText().strip()
task = partial(
self.wallet.verify_message,
- address=address,
+ address=address_e.text(),
signature=str(signature_e.toPlainText()),
- message=message,
+ message=message_e.toPlainText(),
)
def on_result(verified):
### electrum/wallet.py
@@ -3234,10 +3234,14 @@ def update_password(self, old_pw, new_pw, *, encrypt_storage: bool = True, xpub_
def _update_password_for_keystore(self, old_pw: Optional[str], new_pw: Optional[str]) -> None:
pass
- def sign_message(self, *, address: str, message: str, password) -> bytes:
+ def sign_message(self, *, address: str, message: str, password, strip_inputs: bool = True) -> bytes:
"""Caller must handle UserFacingException."""
assert isinstance(address, str), f"address must be str. got {type(address)}"
assert isinstance(message, str), f"message must be str. got {type(message)}"
+ if strip_inputs:
+ # stripping whitespaces leads to better UX for GUIs, but it's counter-productive for CLI
+ address = address.strip()
+ message = message.strip()
if not bitcoin.is_address(address):
raise UserFacingException(_("Invalid Bitcoin address."))
if self.is_watching_only():
@@ -3260,11 +3264,16 @@ def sign_message(self, *, address: str, message: str, password) -> bytes:
return self.keystore.sign_message(index, message, password, script_type=txin_type)
@classmethod
- def verify_message(cls, *, address: str, signature: str, message: str) -> bool:
+ def verify_message(cls, *, address: str, signature: str, message: str, strip_inputs: bool = True) -> bool:
"""Caller must handle UserFacingException."""
assert isinstance(address, str), f"address must be str. got {type(address)}"
assert isinstance(signature, str), f"signature must be str. got {type(signature)}"
assert isinstance(message, str), f"message must be str. got {type(message)}"
+ if strip_inputs:
+ # stripping whitespaces leads to better UX for GUIs, but it's counter-productive for CLI
+ address = address.strip()
+ signature = signature.strip()
+ message = message.strip()
if not is_address(address):
raise UserFacingException(_("Invalid Bitcoin address."))
try:Why this scored 18/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.