From 7269ef9d4e3a2c15bdc89d3717d9848141f715b8 Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Wed, 29 Jul 2026 23:34:26 +0530 Subject: [PATCH 1/2] perf: verify the length crc8, and stop eager work in the record decoders The reassembler acted on the declared length without checking the crc8 that protects it, so one corrupted length byte consumed up to 4092 bytes of good stream. It now resyncs instead, and exposes a resync count. R24.rawTail is encoded on first read rather than at parse time; it has no readers today and dominated parseR24. hexToBytes uses a nibble lookup over code units, keeping the FormatException callers depend on. --- lib/src/framing.dart | 21 ++++++++++++++++++++- lib/src/live.dart | 19 ++++++++++++++++++- lib/src/records.dart | 18 +++++++++++++----- test/decode_guards_test.dart | 13 +++++++++++++ test/framing_test.dart | 29 +++++++++++++++++++++++++++++ 5 files changed, 93 insertions(+), 7 deletions(-) diff --git a/lib/src/framing.dart b/lib/src/framing.dart index d360ca9..22942ac 100644 --- a/lib/src/framing.dart +++ b/lib/src/framing.dart @@ -78,12 +78,20 @@ Frame? parseFrame(Uint8List raw) { /// out of the running buffer. length-based reassembler. class FrameReassembler { final List _buf = []; + int _resyncs = 0; + + /// Number of times the reassembler skipped a byte because the envelope did + /// not hold up (bad SOF, implausible length, or a length field whose crc8 + /// did not match). Callers use this to detect a degraded link — a bad + /// length is discarded here, so it never reaches [Frame.valid]. + int get resyncs => _resyncs; List feed(List chunk) { final out = []; _buf.addAll(chunk); bool resync() { + _resyncs++; // Find next SOF after index 0. int nxt = -1; for (int i = 1; i < _buf.length; i++) { @@ -112,6 +120,14 @@ class FrameReassembler { if (!resync()) break; continue; } + // The crc8 protects the length field and nothing else, so check it + // before acting on `declared`. Skipping this consumes up to 4092 bytes + // of good stream on a single corrupted length byte — records the band + // is about to trim from flash and will not send again. + if (_buf[3] != crc8(Uint8List.fromList([_buf[1], _buf[2]]))) { + if (!resync()) break; + continue; + } if (_buf.length < total) break; // wait for the rest of this frame final frame = parseFrame(Uint8List.fromList(_buf.sublist(0, total))); if (frame != null) out.add(frame); @@ -127,5 +143,8 @@ class FrameReassembler { return out; } - void reset() => _buf.clear(); + void reset() { + _buf.clear(); + _resyncs = 0; + } } diff --git a/lib/src/live.dart b/lib/src/live.dart index d21864b..a8436f5 100644 --- a/lib/src/live.dart +++ b/lib/src/live.dart @@ -60,11 +60,28 @@ class ImuFrame { Map toMap() => {'ts': ts, 'idx': idx, 'mags': mags, 'xs': xs, 'ys': ys, 'zs': zs}; } +/// Nibble value for an ASCII hex code unit, or -1 if it is not a hex digit. +int _nibble(int c) { + if (c >= 0x30 && c <= 0x39) return c - 0x30; // 0-9 + if (c >= 0x61 && c <= 0x66) return c - 0x57; // a-f + if (c >= 0x41 && c <= 0x46) return c - 0x37; // A-F + return -1; +} + Uint8List hexToBytes(String hex) { final trimmed = hex.trim(); final out = Uint8List(trimmed.length ~/ 2); for (int i = 0; i < out.length; i++) { - out[i] = int.parse(trimmed.substring(i * 2, i * 2 + 2), radix: 16); + final hi = _nibble(trimmed.codeUnitAt(i * 2)); + final lo = _nibble(trimmed.codeUnitAt(i * 2 + 1)); + // Keep throwing on non-hex input. Callers rely on it: live.dart, + // substrate.dart, db.dart and ble_engine.dart all treat a FormatException + // as "this string is not a record", and a lookup that returned 0 instead + // would hand them fabricated bytes. + if (hi < 0 || lo < 0) { + throw FormatException('not a hex byte', trimmed, i * 2); + } + out[i] = (hi << 4) | lo; } return out; } diff --git a/lib/src/records.dart b/lib/src/records.dart index 2c354ce..32a84fe 100644 --- a/lib/src/records.dart +++ b/lib/src/records.dart @@ -53,7 +53,13 @@ class R24 { final int ambientRaw; /// Untouched payload [13:] as hex — kept for re-decode as the map improves. - final String rawTail; + /// + /// Encoded on first read, not at parse time. Nothing in the stack reads it + /// today, and eagerly hex-encoding 83 bytes per record dominated parseR24's + /// cost on the offload path. + String get rawTail => _rawTailHex ??= _hexFrom(_rawTailBytes, 0); + final Uint8List _rawTailBytes; + String? _rawTailHex; R24({ required this.histVersion, @@ -71,8 +77,8 @@ class R24 { required this.spo2IrRaw, required this.skinTempRaw, required this.ambientRaw, - required this.rawTail, - }); + required Uint8List rawTailBytes, + }) : _rawTailBytes = rawTailBytes; /// Map matching the TS `out` shape (snake_case keys) for parity comparison. Map toMap() => { @@ -204,7 +210,8 @@ R24? _parseV25(Uint8List inner) { spo2IrRaw: 0, skinTempRaw: 0, ambientRaw: 0, - rawTail: _hexFrom(inner, 13), + rawTailBytes: + inner.length > 13 ? Uint8List.sublistView(inner, 13) : Uint8List(0), ); } @@ -368,7 +375,8 @@ R24? _parseV24Layout( spo2IrRaw: view.getUint16(66, Endian.little), skinTempRaw: view.getUint16(68, Endian.little), ambientRaw: view.getUint16(70, Endian.little), - rawTail: _hexFrom(inner, 13), + rawTailBytes: + inner.length > 13 ? Uint8List.sublistView(inner, 13) : Uint8List(0), ); } diff --git a/test/decode_guards_test.dart b/test/decode_guards_test.dart index 4dd8e13..32020cf 100644 --- a/test/decode_guards_test.dart +++ b/test/decode_guards_test.dart @@ -388,4 +388,17 @@ void main() { expect(() => cmdBuzz(0, 0), returnsNormally); }); }); + + group('hexToBytes', () { + test('decodes both cases and tolerates surrounding whitespace', () { + expect(hexToBytes(' 0aFF10 '), equals([0x0a, 0xff, 0x10])); + expect(hexToBytes(''), isEmpty); + }); + + test('throws on non-hex input instead of fabricating bytes', () { + for (final bad in ['0g', 'zz', '0x', '00 11']) { + expect(() => hexToBytes(bad), throwsFormatException, reason: bad); + } + }); + }); } diff --git a/test/framing_test.dart b/test/framing_test.dart index b5c4c20..06cc749 100644 --- a/test/framing_test.dart +++ b/test/framing_test.dart @@ -148,5 +148,34 @@ void main() { expect(rest.length, 1); expect(rest.first.valid, isTrue); }); + + test('a corrupted length byte does not swallow the frame behind it', () { + final a = buildCommand(3, Cmd.getDataRange, List.filled(64, 0x11)); + final b = buildCommand(4, Cmd.getHelloHarvard, const [0x00]); + final stream = [...a, ...b]; + // Flip one bit in the low length byte. crc8 no longer matches, so the + // declared length must not be trusted. + stream[1] ^= 0x01; + + final re = FrameReassembler(); + final frames = re.feed(stream); + + expect(re.resyncs, greaterThan(0), reason: 'bad length must force a resync'); + expect(frames.length, 1, reason: 'the intact second frame must survive'); + expect(frames.single.valid, isTrue); + expect(frames.single.seq, 4); + }); + + test('a dropped byte costs one frame, not the rest of the stream', () { + final a = buildCommand(5, Cmd.getDataRange, List.filled(32, 0x22)); + final b = buildCommand(6, Cmd.getHelloHarvard, const [0x00]); + final stream = [...a, ...b]..removeAt(2); // drop a length byte + + final re = FrameReassembler(); + final frames = re.feed(stream); + + expect(frames.map((f) => f.seq), contains(6)); + expect(frames.every((f) => f.valid), isTrue); + }); }); } From 511d4fec2e882c7e31d68cd52d209f091331482d Mon Sep 17 00:00:00 2001 From: abdulsaheel Date: Thu, 30 Jul 2026 00:14:02 +0530 Subject: [PATCH 2/2] fix: reject odd-length hex, and let R24 own its tail bytes hexToBytes floored the byte count, so a truncated record decoded cleanly at the wrong length instead of failing. rawTail is encoded lazily now, so holding a view into the caller's buffer meant a later mutation could change bytes that were meant to be a snapshot of parse time. It copies instead; the test fails against the view version. --- lib/src/live.dart | 6 ++++++ lib/src/records.dart | 10 ++++++++-- test/decode_guards_test.dart | 14 +++++++++++++- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/lib/src/live.dart b/lib/src/live.dart index a8436f5..024b154 100644 --- a/lib/src/live.dart +++ b/lib/src/live.dart @@ -70,6 +70,12 @@ int _nibble(int c) { Uint8List hexToBytes(String hex) { final trimmed = hex.trim(); + // An odd-length string is a truncated record, not a shorter one. Flooring to + // whole bytes would drop the trailing nibble and hand the caller a payload + // that decodes cleanly at the wrong length. + if (trimmed.length.isOdd) { + throw FormatException('odd-length hex', trimmed, trimmed.length); + } final out = Uint8List(trimmed.length ~/ 2); for (int i = 0; i < out.length; i++) { final hi = _nibble(trimmed.codeUnitAt(i * 2)); diff --git a/lib/src/records.dart b/lib/src/records.dart index 32a84fe..6e803c4 100644 --- a/lib/src/records.dart +++ b/lib/src/records.dart @@ -210,8 +210,11 @@ R24? _parseV25(Uint8List inner) { spo2IrRaw: 0, skinTempRaw: 0, ambientRaw: 0, + // COPY, not sublistView: a view aliases the caller's buffer, and rawTail + // is now encoded lazily, so a later mutation of `inner` would change bytes + // that were supposed to be a snapshot of parse time. rawTailBytes: - inner.length > 13 ? Uint8List.sublistView(inner, 13) : Uint8List(0), + inner.length > 13 ? Uint8List.fromList(inner.sublist(13)) : Uint8List(0), ); } @@ -375,8 +378,11 @@ R24? _parseV24Layout( spo2IrRaw: view.getUint16(66, Endian.little), skinTempRaw: view.getUint16(68, Endian.little), ambientRaw: view.getUint16(70, Endian.little), + // COPY, not sublistView: a view aliases the caller's buffer, and rawTail + // is now encoded lazily, so a later mutation of `inner` would change bytes + // that were supposed to be a snapshot of parse time. rawTailBytes: - inner.length > 13 ? Uint8List.sublistView(inner, 13) : Uint8List(0), + inner.length > 13 ? Uint8List.fromList(inner.sublist(13)) : Uint8List(0), ); } diff --git a/test/decode_guards_test.dart b/test/decode_guards_test.dart index 32020cf..b3e1b80 100644 --- a/test/decode_guards_test.dart +++ b/test/decode_guards_test.dart @@ -395,8 +395,20 @@ void main() { expect(hexToBytes(''), isEmpty); }); + test('rawTail snapshots the record, even if the buffer is reused', () { + final inner = hexToBytes(_goodV24); + final before = inner[13]; + final rec = parseR24(inner)!; + // rawTail is encoded lazily, so the record must own its bytes: mutating + // the caller's buffer afterwards must not change what it reports. + inner[13] = before ^ 0xFF; + expect(rec.rawTail.substring(0, 2), + before.toRadixString(16).padLeft(2, '0'), + reason: 'rawTail must reflect parse time, not later mutation'); + }); + test('throws on non-hex input instead of fabricating bytes', () { - for (final bad in ['0g', 'zz', '0x', '00 11']) { + for (final bad in ['0g', 'zz', '0x', '00 11', 'a', 'abc']) { expect(() => hexToBytes(bad), throwsFormatException, reason: bad); } });