From 526784448155d94a51fa3981a3583332afa8e1b2 Mon Sep 17 00:00:00 2001 From: kdmukai Date: Thu, 2 Jan 2025 16:40:04 -0600 Subject: [PATCH 1/3] bugfix on edge case Compact SeedQRs --- src/seedsigner/models/decode_qr.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/seedsigner/models/decode_qr.py b/src/seedsigner/models/decode_qr.py index 4ac166423..d21439213 100644 --- a/src/seedsigner/models/decode_qr.py +++ b/src/seedsigner/models/decode_qr.py @@ -417,6 +417,9 @@ def detect_segment_type(s, wordlist_language_code=None): # 32 bytes for 24-word CompactSeedQR; 16 bytes for 12-word CompactSeedQR if len(s) == 32 or len(s) == 16: try: + if not isinstance(s, bytes): + # TODO: remove this check & conversion once above cast to str is removed + s = s.encode() bitstream = "" for b in s: bitstream += bin(b).lstrip('0b').zfill(8) From 75ba1e07ecd9387af40cdf2d55878c1683031900 Mon Sep 17 00:00:00 2001 From: kdmukai Date: Thu, 2 Jan 2025 17:31:34 -0600 Subject: [PATCH 2/3] Further bugfix; convert first, then check length --- src/seedsigner/models/decode_qr.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/seedsigner/models/decode_qr.py b/src/seedsigner/models/decode_qr.py index d21439213..e98a2c561 100644 --- a/src/seedsigner/models/decode_qr.py +++ b/src/seedsigner/models/decode_qr.py @@ -414,12 +414,17 @@ def detect_segment_type(s, wordlist_language_code=None): pass # Is it byte data? + if not isinstance(s, bytes): + try: + # TODO: remove this check & conversion once above cast to str is removed + s = s.encode() + except UnicodeError: + # Couldn't convert back to bytes; shouldn't happen + raise Exception("Conversion to bytes failed") + # 32 bytes for 24-word CompactSeedQR; 16 bytes for 12-word CompactSeedQR if len(s) == 32 or len(s) == 16: try: - if not isinstance(s, bytes): - # TODO: remove this check & conversion once above cast to str is removed - s = s.encode() bitstream = "" for b in s: bitstream += bin(b).lstrip('0b').zfill(8) From cf83ebe033824a360700243103681d853cf9338e Mon Sep 17 00:00:00 2001 From: kdmukai Date: Thu, 2 Jan 2025 17:32:24 -0600 Subject: [PATCH 3/3] add test case --- tests/test_seedqr.py | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/test_seedqr.py b/tests/test_seedqr.py index d792683c1..7a93ac013 100644 --- a/tests/test_seedqr.py +++ b/tests/test_seedqr.py @@ -4,7 +4,6 @@ from seedsigner.models.decode_qr import DecodeQR, DecodeQRStatus from seedsigner.models.encode_qr import SeedQrEncoder, CompactSeedQrEncoder from seedsigner.models.qr_type import QRType -from seedsigner.models.seed import Seed @@ -104,3 +103,28 @@ def test_compact_seedqr_handles_null_bytes(): # 12-word seed, multiple null bytes in a row entropy = os.urandom(10) + b'\x00\x00' + os.urandom(4) run_encode_decode_test(entropy, mnemonic_length=12, qr_type=QRType.SEED__COMPACTSEEDQR) + + +def test_compact_seedqr_bytes_interpretable_as_str(): + """ + Should successfully decode a Compact SeedQR whose bytes can be interpreted as a valid + string. Most Compact SeedQR byte data will raise a UnicodeDecodeError when attempting to + interpret it as a string, but edge cases are possible. + + see: Issue #656 + """ + # Randomly generated to pass the str.decode() step; 12- and 24-word entropy. + entropy_bytes_tests = [ + b'\x00' * 16, # abandon * 11 + about + b'\x12\x15\\1j`3\x0bkL}f\x00ZYK', + b'tv\x1bZjmqN@t\x13\x1aK\\v)', + b'|9\x05\x1aHF9j\xda\xb6v\x05\x08#\x12=', + b"iHK`4\x1a5\xd3\xaf\xd3\xb47htJ.}<\xea\xbf\x88Xh\x01.?R2^\xc2\xb1'", + b'|Z\x11\x1dt\xdd\x97~t&f &G$H|^[\xd3\x9d\x19Z{\ng^', + ] + + for entropy_bytes in entropy_bytes_tests: + entropy_bytes.decode() # should not raise an exception + mnemonic_length = 12 if len(entropy_bytes) == 16 else 24 + run_encode_decode_test(entropy_bytes, mnemonic_length=mnemonic_length, qr_type=QRType.SEED__COMPACTSEEDQR)