Merge pull request #969 from ruipereira1/fix/ur2-bytewords-crc-width
What the developer wrote
Authored by Nick Klockenga
Merge pull request #969 from ruipereira1/fix/ur2-bytewords-crc-width
Serialize the Bytewords CRC as a fixed 4 bytes and re-enable the checksum check
What changed, and why it matters
This commit fixes a bug in how SeedSigner creates and verifies Bytewords checksums, a format used to encode data (such as crypto wallet information) as readable words or compact strings. The checksum was sometimes output as 3 bytes instead of the required 4 bytes, which could cause other wallets or scanners to reject the encoded data. It also re-enables a previously disabled checksum verification, so corrupted data is now properly rejected instead of being silently accepted.
Review whether any previously generated Bytewords frames were produced with the buggy 3-byte CRC and may have been backed up or shared; such frames should be regenerated. Ensure the new tests pass and consider adding a regression test for the exact 3-byte CRC boundary. No immediate remote exploit path is evident, but users should upgrade to a build containing this fix to restore data integrity guarantees.
Security signals we found
Data integrity failure: truncated payload on ~1/256 encodings due to variable-width CRC serialization
Disabled integrity check re-enabled: corrupted Bytewords input was silently accepted
Spec non-compliance: UR/Bytewords requires a fixed 4-byte CRC32
Interoperability risk: spec-compliant decoders would reject or misdecode affected frames
Evidence from the diff
The patch corrects crc32n() in src/seedsigner/helpers/ur2/crc32.py to always serialize the CRC32 checksum as a fixed 4-byte big-endian integer, matching the UR/Bytewords specification. Previously it used n.to_bytes((bit_length(n) + 7) // 8, 'big'), which emitted 3 bytes whenever the CRC value was below 2^24 (roughly 1 in 256 payloads). Because Bytewords.decode() always strips the last 4 bytes as the checksum, a 3-byte checksum caused the decoder to truncate one payload byte. The patch also re-enables the checksum comparison in Bytewords.decode() that had been commented out, restoring integrity checking. New unit tests exercise short-CRC payloads, round-trip behavior, and corruption rejection.
Changed components
src/seedsigner/helpers/ur2/bytewords.pysrc/seedsigner/helpers/ur2/crc32.pytests/test_ur2_bytewords.pyInspect captured patch +67 / −3
### src/seedsigner/helpers/ur2/bytewords.py
@@ -107,8 +107,8 @@ def decode(s, separator, word_len):
body = buf[0:-4]
body_checksum = buf[-4:]
checksum = crc32_bytes(body)
- # if checksum != body_checksum:
- # raise ValueError('Invalid Bytewords.')
+ if checksum != body_checksum:
+ raise ValueError('Invalid Bytewords.')
return body
### src/seedsigner/helpers/ur2/crc32.py
@@ -33,4 +33,8 @@ def crc32(buf):
def crc32n(buf):
n = crc32(buf)
- return n.to_bytes((bit_length(n) + 7) // 8, 'big')
+ # The UR spec's Bytewords checksum is a fixed-width 4-byte big-endian CRC32.
+ # Sizing the output to the value's bit length emits 3 bytes whenever the CRC is
+ # below 2**24 (~1 in 256), which produces frames that spec-compliant decoders
+ # reject and that lax decoders truncate by one payload byte.
+ return n.to_bytes(4, 'big')
### tests/test_ur2_bytewords.py
@@ -0,0 +1,60 @@
+import pytest
+
+from seedsigner.helpers.ur2.bytewords import Bytewords, Bytewords_Style_minimal, Bytewords_Style_standard
+from seedsigner.helpers.ur2.crc32 import crc32, crc32n
+
+
+
+class TestBytewordsChecksum:
+ """
+ The Bytewords checksum is a fixed-width 4-byte big-endian CRC32. Sizing it to the
+ value's bit length emits 3 bytes whenever the CRC is below 2**24 (~1 in 256), and
+ `decode()` always strips 4, so the payload silently loses its last byte.
+ """
+
+ # crc32 of this payload is 0x00b6cdbc, i.e. below 2**24
+ SHORT_CRC_PAYLOAD = bytes.fromhex("9cce484ad8a364ed9360fa24ca015240")
+
+
+ def test_crc32n_is_always_four_bytes(self):
+ assert crc32(self.SHORT_CRC_PAYLOAD) < 2**24, "vector no longer exercises the short-CRC case"
+ assert len(crc32n(self.SHORT_CRC_PAYLOAD)) == 4
+
+ # Also cover a CRC below 2**16
+ for i in range(500_000):
+ candidate = i.to_bytes(4, "big")
+ if crc32(candidate) < 2**16:
+ assert len(crc32n(candidate)) == 4
+ break
+ else:
+ pytest.skip("no sub-2**16 CRC found in the search range")
+
+
+ @pytest.mark.parametrize("style", [Bytewords_Style_minimal, Bytewords_Style_standard])
+ def test_round_trip_with_short_crc(self, style):
+ """The payload must survive a round-trip even when its CRC is small."""
+ encoded = Bytewords.encode(style, self.SHORT_CRC_PAYLOAD)
+ decoded = bytes(Bytewords.decode(style, encoded))
+ assert decoded == self.SHORT_CRC_PAYLOAD
+
+
+ def test_round_trip_sweep(self):
+ """Sweep payload lengths and contents; ~1 in 256 hits the short-CRC path."""
+ import os
+ for _ in range(300):
+ for n in [10, 16, 32, 64]:
+ payload = os.urandom(n)
+ encoded = Bytewords.encode(Bytewords_Style_minimal, payload)
+ assert bytes(Bytewords.decode(Bytewords_Style_minimal, encoded)) == payload
+
+
+ def test_corrupted_payload_is_rejected(self):
+ """The checksum comparison in decode() must actually run."""
+ payload = b"the times 03/Jan/2009"
+ encoded = Bytewords.encode(Bytewords_Style_minimal, payload)
+
+ # Corrupt the first byteword (each byte is 2 chars in the minimal style)
+ corrupted = ("ae" if encoded[0:2] != "ae" else "ad") + encoded[2:]
+
+ with pytest.raises(ValueError):
+ Bytewords.decode(Bytewords_Style_minimal, corrupted)Why this scored 64/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.