From 2476684f24e1a46570188af8109af482c22786cc Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Thu, 24 Sep 2026 00:24:35 +0200 Subject: [PATCH] Charge each listed TLV occurrence against the reassembly cap, and file the unknown-tag gap --- src/pdu.ts | 1 - src/reassembly.ts | 7 ++++++- test/session-extras.test.ts | 17 +++++++++++------ todo.md | 8 ++++++++ 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/src/pdu.ts b/src/pdu.ts index fc3ff60..8ba806f 100644 --- a/src/pdu.ts +++ b/src/pdu.ts @@ -262,7 +262,6 @@ export function objToPdu(obj: PduObjectInput): Result< ); } - function readParams( cmdName: CommandName, pdu: Buffer, diff --git a/src/reassembly.ts b/src/reassembly.ts index a11dc53..a40eccc 100644 --- a/src/reassembly.ts +++ b/src/reassembly.ts @@ -73,6 +73,9 @@ function detach(pduObj: PduObject): PduObject { return { ...pduObj, params, shortMessageOctets: octets, tlvs }; } +// Roughly the heap a listed occurrence costs beyond its value, so a PDU of empty repeats is not free. +const listedTlvOverhead = 200; + // A cstring param arrives as a string, and source_addr alone can carry most of a 1 MiB PDU. function sizeOf(value: ParamValue): number { if (Buffer.isBuffer(value)) return value.length; @@ -88,7 +91,9 @@ function octetsOf(pduObj: PduObject): number { } for (const tlv of Object.values(pduObj.tlvs)) { - octets += tlvOctets(tlv.tagValue); + const listed = Array.isArray(tlv.tagValue) ? tlv.tagValue.length : 0; + + octets += tlvOctets(tlv.tagValue) + listed * listedTlvOverhead; } return octets; diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index 140ae6b..3456adf 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -1824,9 +1824,8 @@ describe('reassembly bounds', () => { assert.equal(collectPayload(40).kept, true); }); - // One segment is 36 octets before its two 10-octet callback numbers. - test('counts every occurrence of a repeatable TLV against the octet cap', () => { - function collectCallbacks(maxOctets: number): Collected { + test('counts every occurrence of a repeatable TLV against the octet cap, empty ones included', () => { + function collectCallbacks(maxOctets: number, tagValue: Buffer[]): Collected { const reassembler = new Reassembler({ log: silentLog, max: 10, @@ -1836,13 +1835,19 @@ describe('reassembly bounds', () => { timeout: 60_000, }); const carried = segment(9, 1, 2); - const tagValue = [Buffer.alloc(10, 0x31), Buffer.alloc(10, 0x32)]; return collectPdu(reassembler, { ...carried, tlvs: { callback_num: { tagId: 0x0381, tagName: 'callback_num', tagValue } } }); } - assert.equal(collectCallbacks(50).kept, false); - assert.equal(collectCallbacks(60).kept, true); + const numbers = [Buffer.alloc(10_000, 0x31), Buffer.alloc(10_000, 0x32)]; + + assert.equal(collectCallbacks(20_000, numbers).kept, false); + assert.equal(collectCallbacks(30_000, numbers).kept, true); + assert.equal( + collectCallbacks(30_000, Array.from({ length: 10_000 }, () => Buffer.alloc(0))).kept, + false, + 'an empty occurrence still holds an object', + ); }); // The segments before it were answered ESME_ROK, so dropping those is not the same as refusing one. diff --git a/todo.md b/todo.md index e6af354..044ed08 100644 --- a/todo.md +++ b/todo.md @@ -6,6 +6,14 @@ hard rules first — they constrain every item below. This is a working file that sets its own rules. The documentation conventions in AGENTS.md do not govern it, and nothing here is a source anything else may cite. +## Security + +- [ ] **Charge a held segment's TLVs for the objects they keep, not only their value octets.** A + peer sending segments that carry thousands of distinct unknown tags with empty values makes + this library hold megabytes of heap per segment that `maxOctets` counts as nothing, up to + 255 segments per group. Repeatable tags are already charged per occurrence. From the stability + review of #25. + ## Status The rewrite is **feature complete and green**: the suite, lint and typecheck are clean on Node 18