mirror of
https://github.com/SeedSigner/seedsigner.git
synced 2026-10-05 15:08:25 +00:00
Serialize the Bytewords CRC as a fixed 4 bytes and re-enable the check
crc32n() sized its output to the CRC value's bit length, so a CRC below 2**24 was emitted as 3 bytes instead of 4 (~1 in 256 payloads; 2 bytes below 2**16). bytewords.decode() unconditionally strips buf[-4:] as the checksum, so a short CRC costs the payload its last byte. This corrupts data the device *emits*. Both UREncoder.encode() (single-part) and UREncoder.encode_part() (fountain frames) go through Bytewords.encode(), so roughly 0.4% of every UR2 QR frame SeedSigner displays is not spec compliant. For a single-part UR the frame is deterministic, so an affected wallet's xpub QR is broken on every export, permanently: measured 2 broken exports out of 600 randomly generated single-sig wallets. A spec-compliant reader rejects the frame on the checksum; a lax reader silently drops the payload's final byte. The bug is silent because the checksum comparison in decode() was commented out. Re-enabled here, which is only possible once crc32n emits a fixed width: otherwise `checksum` (variable length) and `body_checksum` (always buf[-4:]) could not be compared. Note utils.int_to_bytes() already carries the fixed-width form with the bit-length version commented out above it. Vendored from Foundation Devices' ur-py; I have not checked whether upstream shares this.
This commit is contained in:
@@ -107,8 +107,8 @@ def decode(s, separator, word_len):
|
|||||||
body = buf[0:-4]
|
body = buf[0:-4]
|
||||||
body_checksum = buf[-4:]
|
body_checksum = buf[-4:]
|
||||||
checksum = crc32_bytes(body)
|
checksum = crc32_bytes(body)
|
||||||
# if checksum != body_checksum:
|
if checksum != body_checksum:
|
||||||
# raise ValueError('Invalid Bytewords.')
|
raise ValueError('Invalid Bytewords.')
|
||||||
|
|
||||||
return body
|
return body
|
||||||
|
|
||||||
|
|||||||
@@ -33,4 +33,8 @@ def crc32(buf):
|
|||||||
|
|
||||||
def crc32n(buf):
|
def crc32n(buf):
|
||||||
n = crc32(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')
|
||||||
|
|||||||
@@ -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)
|
||||||
Reference in New Issue
Block a user