Charge each held segment for its objects, at the measured cost, and file the held-message gap

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

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);

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,

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);

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