transaction: tx_from_any: follow-up: only rm whitespaces from strings
What the developer wrote
Authored by SomberNight
transaction: tx_from_any: follow-up: only rm whitespaces from strings
What changed, and why it matters
This commit tightens how Electrum cleans up transaction data before parsing. Previously, when given raw transaction bytes, the code would strip out any byte that looked like whitespace (such as tab or newline bytes). That is risky because a real Bitcoin transaction can legitimately contain bytes like 0x0a (which looks like a newline). The fix only removes whitespace from text strings, and only trims leading/trailing whitespace from byte inputs. It also improves an error message so raw bytes are shown safely. This is a defensive hardening change that prevents possible transaction misparsing or crashes, but the commit itself does not claim a specific exploit.
Treat as a security-hardening fix. Review whether prior releases could misparse or reject crafted transactions containing whitespace-like bytes, and consider whether any user-facing parsing failures could be induced by an attacker supplying a transaction with embedded 0x0a/0x09 bytes. No immediate emergency response is indicated, but the fix should be included in the next release.
Security signals we found
Input sanitization bug in transaction deserialization
Potential corruption of byte-serialized transactions via over-broad whitespace stripping
Defensive hardening of parser boundary between text and binary inputs
Error-message improvement using repr to avoid formatting issues with raw bytes
Evidence from the diff
In electrum/transaction.py, convert_raw_tx_to_hex() previously ran re.sub(rb’\s’, b’‘, raw) on bytes-like inputs, removing all bytes matching Python’s \s whitespace class. Because \s includes 0x0a (newline), 0x09 (tab), 0x0c (form feed), etc., and Bitcoin transaction fields such as nVersion or nLockTime can contain those byte values, this could corrupt a valid serialized transaction or cause parsing to fail. The patch changes behavior so bytes-like inputs only get .strip() (leading/trailing whitespace removal), while full in-string whitespace removal is restricted to str inputs. The error message in tx_from_any() now uses !r to safely represent the raw prefix. Tests are updated to reflect that bytes inputs only tolerate leading/trailing whitespace, not internal whitespace.
Changed components
electrum/transaction.py:convert_raw_tx_to_hex()electrum/transaction.py:tx_from_any()tests/test_transaction.pyInspect captured patch +12 / −8
diff --git a/electrum/transaction.py b/electrum/transaction.py
index 5bbbdf8..4d28995 100644
--- a/electrum/transaction.py
+++ b/electrum/transaction.py
@@ -1483,11 +1483,15 @@ def convert_raw_tx_to_hex(raw: Union[str, bytes]) -> str:
if not raw:
raise ValueError("empty string")
raw_unstripped = raw
- # remove all whitespace characters
- if isinstance(raw, str):
+ # try to remove whitespaces.
+ # FIXME we should NOT do *any* whitespace mangling for bytes-like inputs, only str
+ raw = raw.strip() # remove leading/trailing whitespace, even if bytes-like
+ if isinstance(raw, str): # remove all whitespace characters, if str
+ # note: we don't do this for bytes-like inputs, as whitespace-looking bytes can appear
+ # anywhere in a raw tx. Even leading/trailing pseudo-whitespace: consider that
+ # the nVersion or the nLocktime might contain "0a" bytes
+ # consider e.g.: "\n".encode().hex() == "0a"
raw = re.sub(r'\s', '', raw)
- else:
- raw = re.sub(rb'\s', b'', raw)
# try hex
try:
return binascii.unhexlify(raw).hex()
@@ -1529,7 +1533,7 @@ def tx_from_any(raw: Union[str, bytes], *,
return tx
except Exception as e:
raise SerializationError(f"Failed to recognise tx encoding, or to parse transaction. "
- f"raw: {raw[:30]}...") from e
+ f"raw: {raw[:30]!r}...") from e
class PSBTGlobalType(IntEnum):
diff --git a/tests/test_transaction.py b/tests/test_transaction.py
index ed74461..6d10fb6 100644
--- a/tests/test_transaction.py
+++ b/tests/test_transaction.py
@@ -276,11 +276,11 @@ class TestTransaction(ElectrumTestCase):
data = raw_tx.data
tx_from_any(data) # test if raises (should not)
else:
- mid = len(raw_tx.data) // 2
if isinstance(raw_tx.data, str):
+ mid = len(raw_tx.data) // 2 # for str, sprinkle whitespaces all over
data = whitespace_str + raw_tx.data[:mid] + whitespace_str + raw_tx.data[mid:] + whitespace_str
- else:
- data = whitespace_bytes + raw_tx.data[:mid] + whitespace_bytes + raw_tx.data[mid:] + whitespace_bytes
+ else: # bytes only tolerate whitespaces that are leading/trailing
+ data = whitespace_bytes + raw_tx.data + whitespace_bytes
if raw_tx.is_whitespace_allowed:
tx_from_any(data) # test if raises (should not)
else:
Why this scored 37/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.