From 9c28b586bb08a57b80e7a954c2dd3d69b4627636 Mon Sep 17 00:00:00 2001 From: serversdown Date: Thu, 1 Oct 2026 00:41:08 -0400 Subject: [PATCH] fix(micromate): the 0x5A chunk offset is a uint32 -- a 64 KB cap was one event away UM20147 holds a 72,560-byte event. chunk_params() wrote the offset as a uint16 at params[2:4], because every offset THOR was observed to send fits in two bytes (largest 0x3400 = 13,312), which caps a download at 65,536 B -- so that event could not have been fetched at all. params[0:4] is demonstrably ONE 4-byte field: chunk 0 puts the 4-byte event key there. Writing the offset as a uint32 BE in the same slot is BYTE-IDENTICAL for every offset below 65,536, so nothing verified against THOR's 74 captured frames changes -- the replay test still matches all of them -- and the range extends to 4 GB. Above 64 KB this is inference and the docstring says so: the field's width is established, the device's handling of a non-zero high byte is not. Also newly reachable: at offset 1 MiB params[1] is 0x10, which must go out as `10 10`. Nothing below 64 KB can produce that, so the uint32 change is what first makes the case possible -- and an unescaped 0x10 in 5A params is the exact bug that cost the Series III walk a release. Tested. THIS IS THE SERIES III 64 KB PAGE-BOUNDARY BUG WEARING A DIFFERENT HAT. There, parse_strt_end_offset() discards the key's page byte and the walk crashes once a unit's buffer crosses 64 KB; that one is still open. The transferable lesson: an address field whose high bytes are zero in every capture is not a narrow field, it is an untested one. Same mistake, found twice, in code written years apart. Confirmed in the same run: 0x06 content[0:4] IS the event count -- UM20147 holds 5 events and reads 5, making it three for three across both firmware lines (0->0, 6->6, 5->5). And content[4:8] is a CONSTANT, not a count: it reads 9 on a unit with 6 events and on a unit with 5. list_events() still walks to the sentinel; the count is worth adopting as a pre-check, not a replacement. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ru8Lg9HkkYvX9VWWo65SmL --- docs/micromate_protocol_reference.md | 31 +++++++++++++++ micromate/protocol.py | 28 ++++++++++++-- tests/test_micromate_protocol.py | 58 ++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 4 deletions(-) diff --git a/docs/micromate_protocol_reference.md b/docs/micromate_protocol_reference.md index 06ba509..075108d 100644 --- a/docs/micromate_protocol_reference.md +++ b/docs/micromate_protocol_reference.md @@ -994,6 +994,37 @@ its counter resets, so keys are reused *within* a unit, which is why `ach_state.json` tracks `max_downloaded_key` per serial. Series IV inherits that and adds cross-unit collision on top. +### ⚠ 🔑 The chunk offset is a uint32 — and a 64 KB cap was one event away + +UM20147 holds a **72,560-byte** event (`055d4a83`). The `0x5A` chunk offset was +implemented as a **uint16 at `params[2:4]`**, because every offset THOR was +observed to send fits in two bytes — the largest is `0x3400` (13,312). That caps +a download at **65,536 bytes**, so that event could not have been fetched at all. + +`params[0:4]` is demonstrably **one 4-byte field**: chunk 0 puts the 4-byte event +key there. Writing the offset as a uint32 BE in the same slot is **byte-identical +for every offset below 65,536** — so it changes nothing that was verified against +THOR's 74 captured frames — and extends the range to 4 GB. + +⚠ **Above 64 KB this is inference.** No capture exercises a carry into +`params[1]`. The field's width is established; the device's handling of a +non-zero high byte is not. + +⚠ **At offset 1 MiB `params[1]` is `0x10`**, which must be escaped on the wire as +`10 10`. Unreachable below 64 KB, so the uint32 change is what first makes that +case possible — and an unescaped `0x10` in `5A` params is the exact bug that cost +the Series III walk a release. + +**This is the Series III 64 KB page-boundary bug wearing a different hat.** There, +`parse_strt_end_offset()` discards the key's page byte and the walk crashes once a +unit's buffer crosses 64 KB; that is **still open**. The transferable lesson: +*an address field whose high bytes are zero in every capture is not a narrow +field, it is an untested one.* Both bugs are the same mistake, found twice, in +code written years apart. + +**The test is sitting on a desk:** download `055d4a83` from UM20147 and see +whether 71 chunks come back. + ### Still not covered - **A unit that is monitoring**, and a unit with a nearly-full event buffer. - **The inbound call-home session** — still the one protocol unknown. diff --git a/micromate/protocol.py b/micromate/protocol.py index 6e35a2f..6d8ed49 100644 --- a/micromate/protocol.py +++ b/micromate/protocol.py @@ -170,15 +170,35 @@ def chunk_params(key4: bytes, byte_offset: int) -> bytes: """`0x5A`: the key opens the file, then a byte offset walks it. Chunk 0 carries the event key at params[0:4] — that is what says "from the - beginning". Later chunks carry a uint16 BE byte offset at params[2:4]. + beginning". Later chunks carry the byte offset **in that same 4-byte slot**, + as a uint32 BE. + + ⚠ **Written as a uint32 deliberately, and this matters above 64 KB.** Every + offset THOR was observed to send fits in two bytes — the largest was `0x3400` + (13,312) — so `params[0:2]` was always `00 00` and the field looks like a + uint16 at `params[2:4]`. Reading it that way caps a download at **65,536 + bytes**, and UM20147 currently holds a **72,560-byte** event, so that cap is + not hypothetical. + + A uint32 here is **byte-identical for every offset below 65,536**, so it + changes nothing that was verified against THOR's frames (the replay test + asserts all 74 of them) and extends the range to 4 GB. Above 64 KB it is + **inference**: `params[0:4]` is demonstrably one 4-byte field, because chunk 0 + puts a 4-byte key in it, but no capture exercises a carry into `params[1]`. + + ⚠ This is the **Series III 64 KB page-boundary bug in a new guise** — there, + `parse_strt_end_offset()` discards the key's page byte and the `5A` walk + crashes once a unit's buffer crosses 64 KB, which is *still open* on that + side. The lesson that transfers: an address field whose high bytes are zero + in every capture is not a narrow field, it is an untested one. """ if byte_offset == 0: if len(key4) != 4: raise ValueError(f"key4 must be 4 bytes, got {len(key4)}") return key4 + bytes(6) - if not 0 <= byte_offset <= 0xFFFF: - raise ValueError(f"byte_offset must fit in uint16, got {byte_offset}") - return bytes(2) + struct.pack(">H", byte_offset) + bytes(6) + if not 0 <= byte_offset <= 0xFFFFFFFF: + raise ValueError(f"byte_offset must fit in uint32, got {byte_offset}") + return struct.pack(">I", byte_offset) + bytes(6) # ── Protocol ────────────────────────────────────────────────────────────────── diff --git a/tests/test_micromate_protocol.py b/tests/test_micromate_protocol.py index ccb0214..7335957 100644 --- a/tests/test_micromate_protocol.py +++ b/tests/test_micromate_protocol.py @@ -400,3 +400,61 @@ def test_every_captured_download_frame_is_one_we_would_have_sent(): total += len(e["reqs"]) assert total == 56, f"expected 56 download frames across the 6 events, saw {total}" + + +# ── Above 64 KB: the limit UM20147 already exceeds ──────────────────────────── + +def test_chunk_offsets_carry_past_64_kb(): + """⚠ The offset is a uint32 at params[0:4], not a uint16 at params[2:4]. + + Every offset THOR was observed to send fits in two bytes (largest 0x3400), + so params[0:2] was always zero and the field looks narrower than it is. + Reading it as a uint16 caps a download at 65,536 B — and UM20147 holds a + 72,560 B event, so the cap is not hypothetical. + + This is the Series III 64 KB page-boundary bug in a new guise; that one is + still open. An address field whose high bytes are zero in every capture is + not a narrow field, it is an untested one. + """ + key = bytes.fromhex("055d4a83") + # Below the old cap: byte-identical to the uint16 form, so nothing verified + # against THOR's frames changes. + for off in (1024, 13312, 65535): + assert chunk_params(key, off) == bytes(2) + off.to_bytes(2, "big") + bytes(6) + # Above it: the carry lands in params[1]. + assert chunk_params(key, 65536) == bytes.fromhex("00010000") + bytes(6) + assert chunk_params(key, 71680) == bytes.fromhex("00011800") + bytes(6) + + +def test_a_chunk_offset_with_0x10_in_it_is_escaped_on_the_wire(): + """At offset 1 MiB params[1] is 0x10, which must go out as `10 10`. + + Nothing below 64 KB can produce this, so it only became reachable with the + uint32 offset — and an unescaped 0x10 is the bug class that cost the + Series III `5A` walk a release. + """ + params = chunk_params(bytes(4), 1 << 20) + assert params[:4] == bytes.fromhex("00100000") + + from micromate.framing import build_request + frame = build_request(P.SUB_BULK_DOWNLOAD, 0x0400, params) + assert bytes.fromhex("1010") in frame + + +def test_a_72kb_event_downloads_in_71_chunks(): + """UM20147's event 055d4a83, the one that broke the uint16 assumption.""" + size = 72560 + n = 71 + responses = [] + for i in range(n): + want = min(P.CHUNK_SIZE, size - i * P.CHUNK_SIZE) + responses.append(frame(0xA5, bytes(11) + bytes(want))) + + p, t = proto(responses) + got = p.read_event_file(bytes.fromhex("055d4a83"), size) + + assert len(got) == size + assert len(t.written) == n + # Chunk 64 is the first past the old cap; its params must carry the 0x01. + wire = t.written[64] + assert bytes.fromhex("000100") in wire, "the carry into params[1] is on the wire"