From 355bcadb59296d84b94888434356379e2bd4ba17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D0=BD=D1=82=D0=BE=D0=BD=20=D0=A2=D1=80=D0=BE=D1=88?= =?UTF-8?q?=D0=B8=D0=BD?= Date: Fri, 2 Oct 2026 08:40:30 +0300 Subject: [PATCH] v3.10.0 review fixes: repair /api/ram-install-app, escape quotes in esc - /api/ram-install-app referenced undefined spi1/tar (the resolution block was only added to /api/ram-install and /api/cap-compat), so every request answered 500. The same preset/MSL resolution is now in place, with regression tests for both modes (install / make selectable) and for the no-MSL refusal before any card I/O. - esc() now escapes " and ' too: the TAR editor renders free-text descriptions (and other values) into attribute contexts, so a crafted preset import could inject attributes. Three tests asserting the old unescaped output updated; a dedicated esc test added. - _spi1_for_tar names the no-TAR case in its hint (a pre-built packet without tar/spi1 used to report "TAR ?"). - preset free TAR rows no longer store an empty "role" key (the documented shape); a non-list `tars` in an update is refused (400) instead of being silently ignored. Live: the no-MSL request answers 400 with the hint; a make_selectable with a bogus AID runs the format probe and the INSTALL (derived SPI1 16 from the preset MSL), the card answers remote SW 6A88 and the counter advances - no 500. Suites: 658 Python + 775 frontend. No version bump: completes the unreleased v3.10.0 (6360dc5 was never pushed). sw cache simple-v359. --- docs/api.md | 3 +- frontend/index.html | 5 +++- frontend/sw.js | 2 +- frontend/tests/cards_iccid.test.js | 8 ++++- frontend/tests/ram.test.js | 4 +-- pysim_simple_server/presets.py | 7 +++-- pysim_simple_server/server.py | 24 +++++++++++++-- tests/test_fetch_routes.py | 47 ++++++++++++++++++++++++++++++ tests/test_ota_helpers.py | 4 +++ tests/test_presets.py | 10 ++++--- 10 files changed, 99 insertions(+), 15 deletions(-) diff --git a/docs/api.md b/docs/api.md index 8821b28..c7dcacf 100644 --- a/docs/api.md +++ b/docs/api.md @@ -257,7 +257,8 @@ import also accepts presets exported by the older localStorage-based builds. hex are normalised to one form) is refused with `400 {"error": "card with this ICCID already exists: …"}`. - `POST /api/presets/update` — `{"id": "…", "fields": {…}}` (partial update; - a plain `{id, name, …}` body works too, and `keysets` replaces the list). + a plain `{id, name, …}` body works too, `keysets` replaces the list and + `tars` the TAR table — a non-list `tars` value is refused). A counter is written as given — this is the deliberate human edit; unknown ids answer `404`. - `POST /api/presets/delete` — `{"id": "…"}` → `{"ok": true, "removed": true}`. diff --git a/frontend/index.html b/frontend/index.html index abb9f10..621a148 100644 --- a/frontend/index.html +++ b/frontend/index.html @@ -7662,7 +7662,10 @@ async function pysimDetectSameOrigin() { pysimDetectSameOrigin(); function esc(str) { - return str.replace(/&/g, '&').replace(//g, '>'); + // quotes too: the result is used inside attribute values (title, value, + // label) as well as in text contexts + return str.replace(/&/g, '&').replace(//g, '>') + .replace(/"/g, '"').replace(/'/g, '''); } async function pysimFetch(path, body) { diff --git a/frontend/sw.js b/frontend/sw.js index 6b4a8f9..1e148a9 100644 --- a/frontend/sw.js +++ b/frontend/sw.js @@ -1,4 +1,4 @@ -const CACHE = 'simple-v358'; +const CACHE = 'simple-v359'; const URLS = [ 'index.html', 'help.html', diff --git a/frontend/tests/cards_iccid.test.js b/frontend/tests/cards_iccid.test.js index 259a3ff..52c1876 100644 --- a/frontend/tests/cards_iccid.test.js +++ b/frontend/tests/cards_iccid.test.js @@ -58,6 +58,12 @@ function fakeAlert() { return seen; } +test('esc escapes quotes as well as the angle brackets', () => { + // the output is used inside attribute values (value="…", title="…", + // label="…") as well as in text contexts (v3.10.0 review fix) + assert.strictEqual(esc('a"b\'c&'), 'a"b'c<d>&'); +}); + test('cardsNormIccid accepts digits, separators and raw EF hex', () => { assert.strictEqual(cardsNormIccid(DIGITS), DIGITS); assert.strictEqual(cardsNormIccid(' 89 70 1190-0000 4600 098 '), DIGITS); @@ -194,7 +200,7 @@ test('cardsRender highlights the equipped card preset', () => { let rows = els['cards-tbody'].innerHTML.split(''); assert.ok(!/bg-emerald-50/.test(rows[0]), 'the other preset must stay plain'); assert.match(rows[1], /bg-emerald-50/); - assert.match(rows[1], /The equipped card's preset/); + assert.match(rows[1], /The equipped card's preset/); // the same card stored as raw EF hex matches too globalThis.cards[1].iccid = RAW_HEX; cardsRender(); diff --git a/frontend/tests/ram.test.js b/frontend/tests/ram.test.js index 478691d..26d28d4 100644 --- a/frontend/tests/ram.test.js +++ b/frontend/tests/ram.test.js @@ -1443,7 +1443,7 @@ test('ramCompatRowsHtml lists AIDs with names, versions and probe state', () => assert.ok(out.includes('A0000000620101 (javacard.framework) \u2014 1.3'), out); assert.ok(out.includes('1.3 \u2192 0.0 (probe)'), out); assert.ok(out.includes('(appended \u2014 not in the CAP)'), out); - assert.ok(out.includes("(not in the CAP's import list)"), out); + assert.ok(out.includes('(not in the CAP's import list)'), out); assert.ok(out.includes('(uicc.toolkit)'), out); assert.strictEqual(ramCompatRowsHtml([]), ''); }); @@ -1482,7 +1482,7 @@ test('ramCompatFailureHtml names the failing CAP section and the import list', ( }; const out = ramCompatFailureHtml(data, mem); assert.ok(out.includes('Failed in: LOAD (1/2) \u2014 load-file bytes 0\u2013239 / 440 \u00b7 covers Header, Directory, Import \u00b7 completes Import'), out); - assert.ok(out.includes("The card verifies the load file's import list when the Import component is complete (JC VM spec 4.5.2)."), out); + assert.ok(out.includes("The card verifies the load file's import list when the Import component is complete (JC VM spec 4.5.2)."), out); assert.ok(out.includes('remote SW 6985 \u2014 Conditions of use not satisfied'), out); assert.ok(out.includes('Import list under test (6):'), out); assert.ok(out.includes('(javacard.framework)'), out); diff --git a/pysim_simple_server/presets.py b/pysim_simple_server/presets.py index c31711b..0ec07c3 100644 --- a/pysim_simple_server/presets.py +++ b/pysim_simple_server/presets.py @@ -336,7 +336,8 @@ class PresetStore: continue if not (e['tar'] or e['msl'] or e['desc']): continue # a blank editor row - out.append(e) + # free entries carry no `role`: the three roles are the fixed ones + out.append({'tar': e['tar'], 'msl': e['msl'], 'desc': e['desc']}) return out def _normalise_keyset(self, ks): @@ -472,7 +473,9 @@ class PresetStore: merged[k] = fields[k] if isinstance(fields, dict) and 'keysets' in fields: merged['keysets'] = fields['keysets'] - if isinstance(fields, dict) and isinstance(fields.get('tars'), list): + if isinstance(fields, dict) and 'tars' in fields: + if not isinstance(fields['tars'], list): + raise PresetError('tars must be a list of TAR entries') merged['tars'] = fields['tars'] new = self._normalise(merged) new['id'] = cur['id'] diff --git a/pysim_simple_server/server.py b/pysim_simple_server/server.py index ea86d18..ba479f1 100644 --- a/pysim_simple_server/server.py +++ b/pysim_simple_server/server.py @@ -1538,7 +1538,8 @@ def _spi1_for_tar(preset, tar_hex, explicit='', prefer_msl=False): card or send more security than the application expects. Returns ``(spi1, warning)``.""" explicit = str(explicit or '').strip().upper() - msl = presets.tar_msl(preset, tar_hex) + tar = str(tar_hex or '').strip().upper() + msl = presets.tar_msl(preset, tar) if prefer_msl and msl: return msl, '' if explicit: @@ -1546,12 +1547,15 @@ def _spi1_for_tar(preset, tar_hex, explicit='', prefer_msl=False): if msl and _hex_lt(explicit, msl): warning = ('SPI1 %s is below the MSL %s of TAR %s - the card may ' 'answer 0A (insufficient security level)' - % (explicit, msl, tar_hex)) + % (explicit, msl, tar)) return explicit, warning if msl: return msl, '' + if not tar: + raise ValueError('no TAR for the packet and no spi1 given - pass the ' + 'TAR (add its MSL to the card preset) or spi1') raise ValueError('no MSL for TAR %s in the card preset - set it in the ' - 'Cards tab or pass spi1' % (tar_hex or '?')) + 'Cards tab or pass spi1' % tar) # RAM command formats (TS 102 226 5.2.1): the bare C-APDU (compact) or the @@ -7696,6 +7700,20 @@ class PysimHandler(BaseHTTPRequestHandler): privileges=privileges, install_params=install_params_hex, stk_params=stk_params_hex, make_selectable=make_selectable) step_name = 'INSTALL [for install]' + # SCP80 params: the SPI1 comes from the MSL of the packet's + # TAR (the operation uses the preset's table); a caller-supplied + # value is only the fallback for a TAR the preset does not carry. + preset = _preset_by_id(self.server, body.get('preset_id')) + tar = str(body.get('tar') or presets.role_tar(preset, 'isd') + or '000000').strip().upper() + try: + spi1, _ = _spi1_for_tar(preset, tar, body.get('spi1'), + prefer_msl=True) + except ValueError as e: + err = {'success': False, 'error': str(e)} + self._send_json(err, 400) + self._log_resp(err) + return sp_state = { 'spi1': spi1, 'spi2': body.get('spi2', '01'), 'kic': body.get('kic', '25'), 'kid': body.get('kid', '25'), diff --git a/tests/test_fetch_routes.py b/tests/test_fetch_routes.py index 31a5a39..6b77234 100644 --- a/tests/test_fetch_routes.py +++ b/tests/test_fetch_routes.py @@ -737,6 +737,53 @@ class PresetStoreHttpTests(unittest.TestCase): self.assertTrue(captured) self.assertEqual(set(captured), {'1A'}) + def test_an_install_app_operation_derives_the_spi1_from_the_msl(self): + p = self.store.add(self._preset(iccid='8970119000004600098')) + self.store.update(p['id'], {'tars': [ + {'role': 'isd', 'tar': '000000', 'msl': '1A', 'desc': ''}, + {'role': 'uiccRfm', 'tar': 'B00000', 'msl': '16', 'desc': ''}, + {'role': 'usimRfm', 'tar': 'B00001', 'msl': '16', 'desc': ''}]}) + captured = [] + + def fake_step(server, scc, sp, state, name, apdu, silent=False): + captured.append((sp['spi1'], sp['tar'])) + state['steps'].append({'name': name, 'por_status': 'por_ok', 'por_sw': '9000'}) + return True + + patches = [ + mock.patch.object(self.srv, '_ram_detect_format', lambda *a, **k: 'compact'), + mock.patch.object(self.srv, '_ram_send_gp_apdu', fake_step), + ] + for patch in patches: + patch.start() + try: + # INSTALL [for install] + status, resp = self._post('/api/ram-install-app', { + 'mode': 'install', 'loadfile_aid': 'F0414C46416101', + 'module_aid': 'F0414C4641610101', 'preset_id': p['id'], + 'kic': '15', 'kid': '15'}) + self.assertEqual(status, 200, resp) + self.assertTrue(resp['success'], resp) + # INSTALL [for make selectable] + status, resp = self._post('/api/ram-install-app', { + 'mode': 'make_selectable', 'instance_aid': 'F0414C4641610101', + 'preset_id': p['id'], 'kic': '15', 'kid': '15'}) + self.assertEqual(status, 200, resp) + self.assertTrue(resp['success'], resp) + finally: + for patch in reversed(patches): + patch.stop() + self.assertEqual(captured, [('1A', '000000'), ('1A', '000000')]) + + def test_an_install_app_without_an_msl_is_refused_before_the_card(self): + # no preset and no spi1: the resolution must refuse with the hint + # (v3.10.0 review: this endpoint once 500'd on an undefined spi1) + status, resp = self._post('/api/ram-install-app', { + 'mode': 'make_selectable', 'instance_aid': 'F0414C4641610101', + 'kic': '15', 'kid': '15'}) + self.assertEqual(status, 400, resp) + self.assertIn('MSL', resp['error']) + def test_a_counter_less_send_ota_body_still_answers(self): # a pre-built packet without a counter: no final_cntr, no persist, # and above all no int('') crash (v3.8.0 review fix) diff --git a/tests/test_ota_helpers.py b/tests/test_ota_helpers.py index 7c1f588..b35f987 100644 --- a/tests/test_ota_helpers.py +++ b/tests/test_ota_helpers.py @@ -2341,6 +2341,10 @@ class Spi1ForTarTests(unittest.TestCase): self.assertIn('MSL', str(cm.exception)) with self.assertRaises(ValueError): _spi1_for_tar({}, '000000') + # no TAR at all (a pre-built packet without one): the hint names that + with self.assertRaises(ValueError) as cm: + _spi1_for_tar(self.PRESET, '') + self.assertIn('no TAR', str(cm.exception)) class CounterProbeTests(unittest.TestCase): diff --git a/tests/test_presets.py b/tests/test_presets.py index 28de532..8387411 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -110,10 +110,11 @@ class StoreCrudTests(unittest.TestCase): {'tar': 'AF4D02', 'msl': '0A', 'desc': 'My applet'}, {'tar': '', 'msl': '', 'desc': ''}, # a blank editor row ])) - self.assertEqual([(t['role'], t['tar'], t['msl']) for t in p['tars']], + self.assertEqual([(t.get('role', ''), t['tar'], t['msl']) for t in p['tars']], [('isd', 'AF4D01', '1A'), ('uiccRfm', 'B00000', '16'), ('usimRfm', 'B00001', '16'), ('', 'AF4D02', '0A')]) self.assertEqual(p['tars'][3]['desc'], 'My applet') + self.assertNotIn('role', p['tars'][3]) # free rows carry no role self.assertEqual(len(p['tars']), 4) # the blank row was dropped def test_every_tar_needs_its_msl_and_a_unique_tar(self): @@ -161,9 +162,10 @@ class StoreCrudTests(unittest.TestCase): {'role': 'usimRfm', 'tar': 'B00001', 'msl': '16'}]}) self.assertEqual(presets.role_tar(upd, 'isd'), 'AF4D01') self.assertEqual(presets.tar_msl(upd, 'AF4D01'), '1A') - # a malformed tars value must not silently reset the table - again = self.store.update(p['id'], {'tars': 'nonsense'}) - self.assertEqual(presets.role_tar(again, 'isd'), 'AF4D01') + # a malformed tars value is refused, not silently ignored or reset + with self.assertRaises(presets.PresetError): + self.store.update(p['id'], {'tars': 'nonsense'}) + self.assertEqual(presets.role_tar(self.store.get(p['id']), 'isd'), 'AF4D01') def test_a_preset_carries_several_keysets(self): p = self.store.add(preset(keysets=[keyset(kic='15', kid='15'),