From 92456b6f6e7b614967374ee5501cc62eb0b32136 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Fri, 25 Sep 2026 18:57:10 +0200 Subject: [PATCH 1/6] Charge every TLV a held segment carries against the reassembly cap --- CHANGELOG.md | 3 +++ src/reassembly.ts | 6 +++--- test/session-extras.test.ts | 27 +++++++++++++++++---------- todo.md | 8 -------- 4 files changed, 23 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77f326f..01f74d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,9 @@ - An `alert_notification` or an `outbind` from the peer is logged and left unanswered, as SMPP 3.4 gives neither a response. Each one used to emit `sessionError`, `"alert_notification" has no response command`. +- `maxOctets` charges every TLV a held segment carries for the memory it keeps, empty ones + included. A peer could hold megabytes per segment beyond the cap by sending thousands of empty + TLVs, which it counted as nothing. - `server()` refuses a `maxOctets` below 1 or not a whole number, `Infinity` included, like its other limits. `server({ maxOctets: 0 })` used to start and then refuse every multipart message. - `callback_num`, `callback_num_atag`, `callback_num_pres_ind`, `broadcast_area_identifier` and diff --git a/src/reassembly.ts b/src/reassembly.ts index a40eccc..00d0409 100644 --- a/src/reassembly.ts +++ b/src/reassembly.ts @@ -73,8 +73,8 @@ 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; +// Roughly the heap a TLV or listed occurrence costs beyond its value, so a PDU of empty ones is not free. +const tlvObjectOverhead = 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 { @@ -93,7 +93,7 @@ function octetsOf(pduObj: PduObject): number { for (const tlv of Object.values(pduObj.tlvs)) { const listed = Array.isArray(tlv.tagValue) ? tlv.tagValue.length : 0; - octets += tlvOctets(tlv.tagValue) + listed * listedTlvOverhead; + octets += tlvOctets(tlv.tagValue) + (1 + listed) * tlvObjectOverhead; } return octets; diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index 3456adf..4add97b 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -1805,7 +1805,7 @@ describe('reassembly bounds', () => { assert.deepEqual(lost, [], 'the peer holds the only segment there was, so nothing was lost'); }); - // The two addresses are 22 octets, so only the 14 the TLV carries can overrun a cap of 30. + // The two addresses and the TLV's object are 222 octets, so only the 14 it carries can overrun a cap of 230. test('counts a body carried in message_payload against the octet cap', () => { function collectPayload(maxOctets: number): Collected { const reassembler = new Reassembler({ @@ -1820,12 +1820,12 @@ describe('reassembly bounds', () => { return collectPdu(reassembler, payloadSegment(9, 1, 2)); } - assert.equal(collectPayload(30).kept, false, 'a TLV body the cap cannot hold is refused, not dropped later'); - assert.equal(collectPayload(40).kept, true); + assert.equal(collectPayload(230).kept, false, 'a TLV body the cap cannot hold is refused, not dropped later'); + assert.equal(collectPayload(240).kept, true); }); - test('counts every occurrence of a repeatable TLV against the octet cap, empty ones included', () => { - function collectCallbacks(maxOctets: number, tagValue: Buffer[]): Collected { + test('counts every TLV and every occurrence of a repeatable one against the octet cap, empty ones included', () => { + function collectTlvs(maxOctets: number, tlvs: PduObject['tlvs']): Collected { const reassembler = new Reassembler({ log: silentLog, max: 10, @@ -1834,11 +1834,17 @@ describe('reassembly bounds', () => { onLost: () => undefined, timeout: 60_000, }); - const carried = segment(9, 1, 2); - return collectPdu(reassembler, { ...carried, tlvs: { callback_num: { tagId: 0x0381, tagName: 'callback_num', tagValue } } }); + return collectPdu(reassembler, { ...segment(9, 1, 2), tlvs }); } + function collectCallbacks(maxOctets: number, tagValue: Buffer[]): Collected { + return collectTlvs(maxOctets, { callback_num: { tagId: 0x0381, tagName: 'callback_num', tagValue } }); + } + const unknownTags = Object.fromEntries(Array.from({ length: 10_000 }, (_, i) => { + const tagId = 0x4000 + i; + return [String(tagId), { tagId, tagName: undefined, tagValue: Buffer.alloc(0) }]; + })); const numbers = [Buffer.alloc(10_000, 0x31), Buffer.alloc(10_000, 0x32)]; assert.equal(collectCallbacks(20_000, numbers).kept, false); @@ -1848,6 +1854,7 @@ describe('reassembly bounds', () => { false, 'an empty occurrence still holds an object', ); + assert.equal(collectTlvs(30_000, unknownTags).kept, false, 'an empty tag still holds an object'); }); // The segments before it were answered ESME_ROK, so dropping those is not the same as refusing one. @@ -1921,9 +1928,9 @@ describe('reassembly bounds', () => { assert.equal(counted.size, 1, 'the second group evicted the first, as a UDH group would'); counted.clear(); - // A sar_* segment is the two 11-octet addresses plus an 8-octet body, with no UDH to carry. - assert.equal(collectSar(capped(20), 3, 1, 2).kept, false); - assert.equal(collectSar(capped(30), 3, 1, 2).kept, true); + // A sar_* segment is the two 11-octet addresses, an 8-octet body and three 200-octet TLV objects. + assert.equal(collectSar(capped(620), 3, 1, 2).kept, false); + assert.equal(collectSar(capped(630), 3, 1, 2).kept, true); }); // Nothing else says a message the peer has already been answered for was thrown away. diff --git a/todo.md b/todo.md index 7b83a5f..843a501 100644 --- a/todo.md +++ b/todo.md @@ -6,14 +6,6 @@ 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 -- 2.52.0 From f9c53aa576d00c1ac7a05d8cdbb4a60319f4ea25 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Fri, 25 Sep 2026 19:02:07 +0200 Subject: [PATCH 2/6] Charge each held segment for its objects, at the measured cost, and file the held-message gap --- CHANGELOG.md | 6 +++--- src/reassembly.ts | 7 ++++--- test/session-extras.test.ts | 28 ++++++++++++++++------------ test/session.test.ts | 4 ++-- todo.md | 8 ++++++++ 5 files changed, 33 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01f74d3..f5857a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,9 +35,9 @@ - An `alert_notification` or an `outbind` from the peer is logged and left unanswered, as SMPP 3.4 gives neither a response. Each one used to emit `sessionError`, `"alert_notification" has no response command`. -- `maxOctets` charges every TLV a held segment carries for the memory it keeps, empty ones - included. A peer could hold megabytes per segment beyond the cap by sending thousands of empty - TLVs, which it counted as nothing. +- `maxOctets` charges each held segment, and every TLV it carries, for the memory it keeps beyond + its octets. A peer could hold around 67 times the cap with segments of empty fields, and megabytes + per segment with thousands of empty TLVs, both of which the cap counted as next to nothing. - `server()` refuses a `maxOctets` below 1 or not a whole number, `Infinity` included, like its other limits. `server({ maxOctets: 0 })` used to start and then refuse every multipart message. - `callback_num`, `callback_num_atag`, `callback_num_pres_ind`, `broadcast_area_identifier` and diff --git a/src/reassembly.ts b/src/reassembly.ts index 00d0409..c53d993 100644 --- a/src/reassembly.ts +++ b/src/reassembly.ts @@ -73,8 +73,9 @@ function detach(pduObj: PduObject): PduObject { return { ...pduObj, params, shortMessageOctets: octets, tlvs }; } -// Roughly the heap a TLV or listed occurrence costs beyond its value, so a PDU of empty ones is not free. -const tlvObjectOverhead = 200; +// Measured heap beyond the octets, so a segment of empty fields or empty TLVs is not free. +const segmentObjectOverhead = 1000; +const tlvObjectOverhead = 300; // A cstring param arrives as a string, and source_addr alone can carry most of a 1 MiB PDU. function sizeOf(value: ParamValue): number { @@ -84,7 +85,7 @@ function sizeOf(value: ParamValue): number { } function octetsOf(pduObj: PduObject): number { - let octets = 0; + let octets = segmentObjectOverhead; for (const value of Object.values(pduObj.params)) { octets += sizeOf(value); diff --git a/test/session-extras.test.ts b/test/session-extras.test.ts index 4add97b..d4b4314 100644 --- a/test/session-extras.test.ts +++ b/test/session-extras.test.ts @@ -1805,7 +1805,7 @@ describe('reassembly bounds', () => { assert.deepEqual(lost, [], 'the peer holds the only segment there was, so nothing was lost'); }); - // The two addresses and the TLV's object are 222 octets, so only the 14 it carries can overrun a cap of 230. + // The two addresses and the segment's and TLV's objects are 1322 octets, so only the 14 it carries can overrun 1330. test('counts a body carried in message_payload against the octet cap', () => { function collectPayload(maxOctets: number): Collected { const reassembler = new Reassembler({ @@ -1820,11 +1820,11 @@ describe('reassembly bounds', () => { return collectPdu(reassembler, payloadSegment(9, 1, 2)); } - assert.equal(collectPayload(230).kept, false, 'a TLV body the cap cannot hold is refused, not dropped later'); - assert.equal(collectPayload(240).kept, true); + assert.equal(collectPayload(1330).kept, false, 'a TLV body the cap cannot hold is refused, not dropped later'); + assert.equal(collectPayload(1340).kept, true); }); - test('counts every TLV and every occurrence of a repeatable one against the octet cap, empty ones included', () => { + test('counts the objects a segment and each of its TLVs hold against the octet cap, empty ones included', () => { function collectTlvs(maxOctets: number, tlvs: PduObject['tlvs']): Collected { const reassembler = new Reassembler({ log: silentLog, @@ -1854,7 +1854,11 @@ describe('reassembly bounds', () => { false, 'an empty occurrence still holds an object', ); - assert.equal(collectTlvs(30_000, unknownTags).kept, false, 'an empty tag still holds an object'); + // The segment itself is 1036 octets, so the tags must be charged 300 each to overrun 3,001,000. + assert.equal(collectTlvs(3_001_000, unknownTags).kept, false, 'an empty tag still holds an object'); + assert.equal(collectTlvs(3_002_000, unknownTags).kept, true); + assert.equal(collectTlvs(1_000, {}).kept, false, 'a segment holds objects beyond its 36 octets'); + assert.equal(collectTlvs(1_036, {}).kept, true); }); // The segments before it were answered ESME_ROK, so dropping those is not the same as refusing one. @@ -1863,8 +1867,8 @@ describe('reassembly bounds', () => { const reassembler = new Reassembler({ log: silentLog, max: 10, - // One segment is 36 octets, so the second overruns a group already holding the first. - maxOctets: 50, + // One segment is 1036 octets, so the second overruns a group already holding the first. + maxOctets: 1050, now: () => 0, onLost: one => { lost.push(one); }, timeout: 60_000, @@ -1928,9 +1932,9 @@ describe('reassembly bounds', () => { assert.equal(counted.size, 1, 'the second group evicted the first, as a UDH group would'); counted.clear(); - // A sar_* segment is the two 11-octet addresses, an 8-octet body and three 200-octet TLV objects. - assert.equal(collectSar(capped(620), 3, 1, 2).kept, false); - assert.equal(collectSar(capped(630), 3, 1, 2).kept, true); + // A sar_* segment is the two 11-octet addresses, an 8-octet body, its own object and three TLVs'. + assert.equal(collectSar(capped(1920), 3, 1, 2).kept, false); + assert.equal(collectSar(capped(1930), 3, 1, 2).kept, true); }); // Nothing else says a message the peer has already been answered for was thrown away. @@ -2046,8 +2050,8 @@ describe('reassembly bounds', () => { const reassembler = new Reassembler({ log: silentLog, max: 10, - // One segment is 36 octets: 14 of short_message plus the two 11-octet addresses. - maxOctets: 80, + // One segment is 1036 octets: 14 of short_message, the two 11-octet addresses and its object. + maxOctets: 2100, now: () => 0, onLost: () => undefined, timeout: 60_000, diff --git a/test/session.test.ts b/test/session.test.ts index 5e936ae..9aab5e6 100644 --- a/test/session.test.ts +++ b/test/session.test.ts @@ -910,8 +910,8 @@ describe('receiving', () => { // The refusal a submission gets is the one submit_sm_resp defines, whichever command carried it. test('refuses a data_sm segment a server has no room for with the submit code', async t => { - // The two addresses are 22 octets, so the 6-octet UDH and its text are what overrun 30. - const smpp = await startServer(t, { maxOctets: 30 }); + // The two addresses and the objects are 1322 octets, so the 6-octet UDH and its text are what overrun 1330. + const smpp = await startServer(t, { maxOctets: 1330 }); const { session } = await connect(t, smpp, { bindType: 'transmitter' }); assert.ok(session); diff --git a/todo.md b/todo.md index 843a501..2650bab 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 + +- [ ] **Bound the heap `HeldMessages` keeps, not only its count.** It holds up to 1000 messages the + application has not answered for up to 300 s, each as the PDUs it arrived in, undetached and + uncharged: one 200 KB PDU of 50,000 empty unknown TLVs is 15 MB of heap, and a completed + reassembly is up to `maxOctets`. A peer faster than an application answering asynchronously + holds gigabytes per session. From the stability review of #27. + ## Status The rewrite is **feature complete and green**: the suite, lint and typecheck are clean on Node 18 -- 2.52.0 From fd6ce1632588efca1ddb8815b3cac16ce076effa Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Fri, 25 Sep 2026 19:04:28 +0200 Subject: [PATCH 3/6] Drop a false multiple from the changelog, and name the new per-segment charge --- CHANGELOG.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5857a8..822a669 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,9 +35,10 @@ - An `alert_notification` or an `outbind` from the peer is logged and left unanswered, as SMPP 3.4 gives neither a response. Each one used to emit `sessionError`, `"alert_notification" has no response command`. -- `maxOctets` charges each held segment, and every TLV it carries, for the memory it keeps beyond - its octets. A peer could hold around 67 times the cap with segments of empty fields, and megabytes - per segment with thousands of empty TLVs, both of which the cap counted as next to nothing. +- `maxOctets` charges each held segment about 1 KB, and every TLV it carries 300 octets, for the + memory they keep beyond their octets. Segments of empty fields or thousands of empty TLVs used to + count as next to nothing, so a peer could hold far more than the cap. A `maxOctets` tuned low now + holds fewer segments. - `server()` refuses a `maxOctets` below 1 or not a whole number, `Infinity` included, like its other limits. `server({ maxOctets: 0 })` used to start and then refuse every multipart message. - `callback_num`, `callback_num_atag`, `callback_num_pres_ind`, `broadcast_area_identifier` and -- 2.52.0 From f1ef6b66434849cb236f6dee2a0189d25f173cb4 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Fri, 25 Sep 2026 19:05:48 +0200 Subject: [PATCH 4/6] State the octet cap's charge as a rule a consumer can size with --- CHANGELOG.md | 9 +++++---- README.md | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 822a669..5cbf5d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,10 +35,11 @@ - An `alert_notification` or an `outbind` from the peer is logged and left unanswered, as SMPP 3.4 gives neither a response. Each one used to emit `sessionError`, `"alert_notification" has no response command`. -- `maxOctets` charges each held segment about 1 KB, and every TLV it carries 300 octets, for the - memory they keep beyond their octets. Segments of empty fields or thousands of empty TLVs used to - count as next to nothing, so a peer could hold far more than the cap. A `maxOctets` tuned low now - holds fewer segments. +- `maxOctets` charges each held segment 1000 octets beyond its own, and each TLV on it, every repeat + included, 300 more. Segments of empty fields or thousands of empty TLVs used to count as next to + nothing, so a peer could hold far more than the cap. **Raise a `maxOctets` you tuned low**: it now + holds a sixth to a tenth as many segments, and an incomplete message evicted over the cap is lost, + since its segments were already answered. - `server()` refuses a `maxOctets` below 1 or not a whole number, `Infinity` included, like its other limits. `server({ maxOctets: 0 })` used to start and then refuse every multipart message. - `callback_num`, `callback_num_atag`, `callback_num_pres_ind`, `broadcast_area_identifier` and diff --git a/README.md b/README.md index d93ca5e..f77a711 100644 --- a/README.md +++ b/README.md @@ -261,7 +261,7 @@ All optional. Timeouts are milliseconds. | `tls` | `false` | A `tls.TlsOptions` object with your certificate and key. A bare `true` is refused. | | `idleTimeout` | `40000` | Drop a peer that has been silent this long. | | `maxReassembly` | `1000` | Incomplete multipart messages held per session. | -| `maxOctets` | `67108864` | Roughly the memory incomplete multipart messages may hold per session. | +| `maxOctets` | `67108864` | Roughly the memory incomplete multipart messages may hold per session: each held segment counts its octets plus 1000, and each TLV on it, every repeat included, 300 more. | | `reassemblyTimeout` | `300000` | How long a late segment can still join an incomplete message. | | `responseTimeout`, `shutdownTimeout`, `maxOutstanding`, `log`, `signal` | as for the client | | -- 2.52.0 From 72b400e661a19d5c329b05d057f55979bcd517b5 Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Fri, 25 Sep 2026 19:06:20 +0200 Subject: [PATCH 5/6] Name the per-occurrence charge exactly --- CHANGELOG.md | 6 +++--- README.md | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cbf5d4..85c0fde 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,9 +35,9 @@ - An `alert_notification` or an `outbind` from the peer is logged and left unanswered, as SMPP 3.4 gives neither a response. Each one used to emit `sessionError`, `"alert_notification" has no response command`. -- `maxOctets` charges each held segment 1000 octets beyond its own, and each TLV on it, every repeat - included, 300 more. Segments of empty fields or thousands of empty TLVs used to count as next to - nothing, so a peer could hold far more than the cap. **Raise a `maxOctets` you tuned low**: it now +- `maxOctets` charges each held segment 1000 octets beyond its own, 300 more per TLV on it, and 300 + per occurrence of a repeatable one. Segments of empty fields or thousands of empty TLVs used to + count as next to nothing, so a peer could hold far more than the cap. **Raise a `maxOctets` you tuned low**: it now holds a sixth to a tenth as many segments, and an incomplete message evicted over the cap is lost, since its segments were already answered. - `server()` refuses a `maxOctets` below 1 or not a whole number, `Infinity` included, like its diff --git a/README.md b/README.md index f77a711..ab71fd2 100644 --- a/README.md +++ b/README.md @@ -261,7 +261,7 @@ All optional. Timeouts are milliseconds. | `tls` | `false` | A `tls.TlsOptions` object with your certificate and key. A bare `true` is refused. | | `idleTimeout` | `40000` | Drop a peer that has been silent this long. | | `maxReassembly` | `1000` | Incomplete multipart messages held per session. | -| `maxOctets` | `67108864` | Roughly the memory incomplete multipart messages may hold per session: each held segment counts its octets plus 1000, and each TLV on it, every repeat included, 300 more. | +| `maxOctets` | `67108864` | Roughly the memory incomplete multipart messages may hold per session: each held segment counts its octets plus 1000, 300 more per TLV on it, and 300 per occurrence of a repeatable one. | | `reassemblyTimeout` | `300000` | How long a late segment can still join an incomplete message. | | `responseTimeout`, `shutdownTimeout`, `maxOutstanding`, `log`, `signal` | as for the client | | -- 2.52.0 From d8d0947e1f5c4548ba2262fb4864d4ed31ce39af Mon Sep 17 00:00:00 2001 From: Lilleman auf Larv Date: Fri, 25 Sep 2026 19:07:11 +0200 Subject: [PATCH 6/6] Drop an edge-false fraction from the changelog --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85c0fde..ae4e243 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,9 +37,9 @@ response command`. - `maxOctets` charges each held segment 1000 octets beyond its own, 300 more per TLV on it, and 300 per occurrence of a repeatable one. Segments of empty fields or thousands of empty TLVs used to - count as next to nothing, so a peer could hold far more than the cap. **Raise a `maxOctets` you tuned low**: it now - holds a sixth to a tenth as many segments, and an incomplete message evicted over the cap is lost, - since its segments were already answered. + count as next to nothing, so a peer could hold far more than the cap. **Raise a `maxOctets` you + tuned low**: it now holds several times fewer segments, and an incomplete message evicted over + the cap is lost, since its segments were already answered. - `server()` refuses a `maxOctets` below 1 or not a whole number, `Infinity` included, like its other limits. `server({ maxOctets: 0 })` used to start and then refuse every multipart message. - `callback_num`, `callback_num_atag`, `callback_num_pres_ind`, `broadcast_area_identifier` and -- 2.52.0