Fix Unicode values in external settings

ports/stm32/boards/Passport/modules/errors.py

@@ -27,4 +27,5 @@
     'PSBT_OVERSIZED',
     'QR_TOO_LARGE',
     'FIRMWARE_UPDATE_FAILED',
+    'USER_SETTINGS_SAVE_FAILED',
 )

ports/stm32/boards/Passport/modules/ext_settings.py

@@ -29,6 +29,10 @@
 from public_constants import DEVICE_SETTINGS
 
 
+class SettingsOutOfSpace(RuntimeError):
+    pass
+
+
 class ExtSettings:
     """Settings stored in external flash, with a secondary backup"""
 
@@ -405,36 +409,37 @@ def do_save(self, erase_old_pos=True):
         # print('do_save({})'.format(erase_old_pos))
         # render as JSON, encrypt and write it.
         self.current['_revision'] = self.current.get('_revision', 1) + 1
+        data = self._serialize_current()
 
         _, pos = self.find_spot(self.my_pos)
-        self.save_impl(pos, erase_old_pos=erase_old_pos)
+        self.save_impl(pos, data=data, erase_old_pos=erase_old_pos)
 
         # print('save(): sf={}, pos={}'.format(sf, pos))
 
-    def save_impl(self, pos, erase_old_pos=True):
+    def _serialize_current(self):
+        d = ujson.dumps(self.current).encode('utf8')
+        if len(d) > self.max_json_len:
+            raise SettingsOutOfSpace('JSON data is larger than {} bytes.'.format(self.max_json_len))
+        return d
+
+    def save_impl(self, pos, data, erase_old_pos=True):
         aes = self.get_aes(pos)
 
+        pad_len = self.max_json_len - len(data)
+
         with SFFile(pos, pre_erased=True, max_size=self.slot_size) as fd:
             chk = trezorcrypto.sha256()
 
             # first the json data
-            d = ujson.dumps(self.current)
             # print('pos: {}'.format(pos))
             # print('current: {}'.format(self.current))
-            # print('data: {}'.format(bytes_to_hex_str(d)))
+            # print('data: {}'.format(bytes_to_hex_str(data)))
 
             # pad w/ zeros
-            data_len = len(d)
-            pad_len = self.max_json_len - data_len
-            if pad_len < 0:
-                # print('ERROR: JSON data is too big!')
-                return
-
-            fd.write(aes.encrypt(d))
-            chk.update(d)
-            del d
+            fd.write(aes.encrypt(data))
+            chk.update(data)
 
-            # print('data_len={} pad_len={}'.format(data_len, pad_len))
+            # print('pad_len={}'.format(pad_len))
 
             while pad_len > 0:
                 here = min(32, pad_len)

ports/stm32/boards/Passport/modules/tasks/save_multisig_wallet_task.py

@@ -14,6 +14,7 @@ async def save_multisig_wallet_task(on_done, ms):
     # Data to save: Important that this fails immediately when Settings memory would overflow
     from common import settings
     from errors import Error
+    from ext_settings import SettingsOutOfSpace
 
     obj = ms.serialize()
 
@@ -32,9 +33,8 @@ async def save_multisig_wallet_task(on_done, ms):
     # Save now, rather than in background, so we can recover from out-of-space situation
     try:
         settings.save()
-        await on_done(None)
-    except BaseException:
-        # Back out change -- User settings doesn't have enough space for this update
+    except BaseException as exc:
+        # Back out the in-memory change when the save fails for any reason.
         try:
             settings.set('multisig', original)
             settings.save()
@@ -43,4 +43,10 @@ async def save_multisig_wallet_task(on_done, ms):
             # Give up on recovery
             pass
 
-        await on_done(Error.USER_SETTINGS_FULL)
+        if isinstance(exc, SettingsOutOfSpace):
+            await on_done(Error.USER_SETTINGS_FULL)
+        else:
+            await on_done(Error.USER_SETTINGS_SAVE_FAILED)
+        return
+
+    await on_done(None)

ports/stm32/boards/Passport/modules/tests/unit/ext_settings.py

@@ -4,8 +4,74 @@
 #
 # Test the external settings module.
 
-from ext_settings import ExtSettings
+import common
+import ujson
 
-settings = ExtSettings()
+from ext_settings import ExtSettings, SettingsOutOfSpace
+
+
+class FakeFlash:
+    def __init__(self, size):
+        self.data = bytearray(b'\xff' * size)
+
+    def read(self, address, buf):
+        buf[:] = self.data[address:address + len(buf)]
+
+    def write(self, address, buf):
+        for i in range(len(buf)):
+            self.data[address + i] &= buf[i]
+
+    def wait_done(self):
+        pass
+
+    def is_busy(self):
+        return False
+
+    def sector_erase(self, address):
+        self.data[address:address + 4096] = b'\xff' * 4096
+
+
+SLOT_SIZE = 512
+SLOT_START = 4096
+FLASH_SIZE = SLOT_START + (SLOT_SIZE * 2)
+SLOTS = range(SLOT_START, FLASH_SIZE, SLOT_SIZE)
+
+common.sf = FakeFlash(FLASH_SIZE)
+
+settings = ExtSettings(slots=SLOTS, slot_size=SLOT_SIZE)
+names = ['\ube44\ud2b8\ucf54\uc778 \uae08\uace0', 'Multisig \u2018Vault\u2019']
+settings.set('multisig', [{'name': name} for name in names])
+settings.save()
+
+loaded = ExtSettings(slots=SLOTS, slot_size=SLOT_SIZE)
+loaded.load()
+assert [entry['name'] for entry in loaded.get('multisig')] == names
+
+# A payload that exactly fills the encoded data area must still round-trip.
+common.sf = FakeFlash(FLASH_SIZE)
+exact = ExtSettings(slots=SLOTS, slot_size=SLOT_SIZE)
+exact.current['value'] = ''
+json_overhead = len(ujson.dumps(exact.current).encode('utf8'))
+exact_value = 'x' * (exact.max_json_len - json_overhead)
+exact.current['value'] = exact_value
+exact.save()
+
+loaded = ExtSettings(slots=SLOTS, slot_size=SLOT_SIZE)
+loaded.load()
+assert loaded.get('value') == exact_value
+
+# An oversized payload must fail before selecting or writing a slot.
+common.sf = FakeFlash(FLASH_SIZE)
+oversized = ExtSettings(slots=SLOTS, slot_size=SLOT_SIZE)
+oversized.current['value'] = exact_value + 'x'
+
+try:
+    oversized.save()
+except SettingsOutOfSpace:
+    pass
+else:
+    raise RuntimeError('Oversized settings should fail before writing')
+
+assert common.sf.data == bytearray(b'\xff' * FLASH_SIZE)
 
 return_value.write(b'OK')