diff --git a/README.md b/README.md index d915d6b..b5e5e4c 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ A self-hosted, full-stack network management platform for MikroTik devices. Monitor, configure, and manage your entire MikroTik infrastructure — routers, switches, and wireless access points — from a single web interface. -![Version](https://img.shields.io/badge/version-0.17.5_Beta-blue) +![Version](https://img.shields.io/badge/version-0.17.6_Beta-blue) ![License](https://img.shields.io/badge/license-AGPLv3-blue) ![Docker](https://img.shields.io/badge/docker-compose-2496ED?logo=docker&logoColor=white) ![TypeScript](https://img.shields.io/badge/TypeScript-5.3-3178C6?logo=typescript&logoColor=white) diff --git a/backend/package.json b/backend/package.json index 6a0a086..8de973c 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-backend", - "version": "0.17.5-beta", + "version": "0.17.6-beta", "description": "Mikrotik Network Management Platform - Backend API", "main": "dist/index.js", "scripts": { diff --git a/backend/src/services/netflow/__tests__/decoder.test.ts b/backend/src/services/netflow/__tests__/decoder.test.ts index fb3c72c..bad03ac 100644 --- a/backend/src/services/netflow/__tests__/decoder.test.ts +++ b/backend/src/services/netflow/__tests__/decoder.test.ts @@ -269,3 +269,84 @@ describe('IPFIX decoding', () => { expect(result.flows[0].srcAddr).toMatch(/^fd00:/); }); }); + +describe('malformed / hostile packets', () => { + // Regression: a template field of length 0 made a record zero-width, so the + // data-set loop never advanced and spun forever — a single unauthenticated UDP + // packet could hang the whole (single-threaded) backend. If this regresses the + // decode call never returns, so these tests hang rather than fail — that is + // still a loud CI failure, and the cache assertions pin the actual guard. + it('rejects a template containing a zero-length field instead of looping forever', () => { + const cache: TemplateCache = new Map(); + const packet = ipfixPacket(1, [ + flowset(2, templateBody(256, [{ id: 1, length: 0 }])), + flowset(256, Buffer.alloc(4)), + ]); + + const result = decodePacket(packet, '10.0.0.9', cache); + expect(result.templatesParsed).toBe(0); + expect(cache.size).toBe(0); + expect(result.flows).toHaveLength(0); + }); + + it('rejects the whole template when a zero-length field sits among valid ones', () => { + const cache: TemplateCache = new Map(); + const packet = ipfixPacket(1, [ + flowset(2, templateBody(257, [ + { id: 8, length: 4 }, + { id: 12, length: 0 }, // malformed + { id: 1, length: 4 }, + ])), + ]); + + expect(decodePacket(packet, '10.0.0.9', cache).templatesParsed).toBe(0); + expect(cache.size).toBe(0); + }); + + it('terminates on a variable-length field whose length byte is zero', () => { + const cache: TemplateCache = new Map(); + const packet = ipfixPacket(1, [ + flowset(2, templateBody(258, [{ id: 1, length: 0xffff }])), + flowset(258, Buffer.alloc(8)), // every length byte reads as 0 + ]); + + const result = decodePacket(packet, '10.0.0.9', cache); + expect(result.templatesParsed).toBe(1); + expect(result.flows).toHaveLength(0); + }); + + it('handles absurd field lengths and truncated records without throwing', () => { + const cache: TemplateCache = new Map(); + const huge = ipfixPacket(1, [ + flowset(2, templateBody(259, [{ id: 1, length: 60000 }])), + flowset(259, Buffer.alloc(8)), + ]); + expect(() => decodePacket(huge, '10.0.0.9', cache)).not.toThrow(); + + // Declares a 16-byte IPv6 field but supplies only 4 bytes of data + const truncated = ipfixPacket(1, [ + flowset(2, templateBody(260, [{ id: 27, length: 16 }, { id: 1, length: 4 }])), + flowset(260, Buffer.alloc(4)), + ]); + const result = decodePacket(truncated, '10.0.0.9', cache); + expect(result.flows).toHaveLength(0); + }); + + it('still decodes valid traffic after rejecting a malformed template', () => { + const cache: TemplateCache = new Map(); + decodePacket( + ipfixPacket(1, [flowset(2, templateBody(256, [{ id: 1, length: 0 }]))]), + '10.0.0.9', + cache + ); + + const good = v9Packet(0, [ + flowset(0, templateBody(256, V9_FIELDS)), + flowset(256, record('192.168.1.10', '1.1.1.1', 50000, 53, 17, 120, 2)), + ]); + const result = decodePacket(good, '10.0.0.9', cache); + expect(result.templatesParsed).toBe(1); + expect(result.flows).toHaveLength(1); + expect(result.flows[0].bytes).toBe(120); + }); +}); diff --git a/backend/src/services/netflow/decoder.ts b/backend/src/services/netflow/decoder.ts index 008282a..9380162 100644 --- a/backend/src/services/netflow/decoder.ts +++ b/backend/src/services/netflow/decoder.ts @@ -106,10 +106,13 @@ function parseTemplates( id = id & 0x7fff; enterprise = true; } + // A zero-length field is meaningless in v9/IPFIX and makes a record + // zero-width, which would spin parseDataSet forever. Reject the template. + if (length === 0) { ok = false; break; } fields.push({ id, length, enterprise }); minLength += length === 0xffff ? 1 : length; } - if (!ok) break; + if (!ok || minLength === 0) break; cache.set(`${exporterKey}|${sourceId}|${templateId}`, { fields, minLength }); parsed++; } @@ -126,6 +129,7 @@ function parseDataSet( let off = start; // Records are packed back to back; trailing bytes < minLength are padding while (off + template.minLength <= setEnd) { + const recordStart = off; let srcAddr = ''; let dstAddr = ''; let srcPort = 0; @@ -167,6 +171,10 @@ function parseDataSet( } if (!valid) break; + // Forward-progress guard: a record that consumed no bytes would loop forever. + // Template validation should prevent this, but never let a malformed packet + // hang the (single-threaded) event loop. + if (off <= recordStart) break; // Only emit records that look like flows (options/stats records lack these) if (srcAddr && dstAddr && bytes >= 0) { flows.push({ srcAddr, dstAddr, srcPort, dstPort, protocol, bytes, packets }); diff --git a/frontend/package.json b/frontend/package.json index 3c6851b..5028086 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-frontend", - "version": "0.17.5-beta", + "version": "0.17.6-beta", "private": true, "type": "module", "scripts": { diff --git a/frontend/src/version.ts b/frontend/src/version.ts index 100dadb..29c2567 100644 --- a/frontend/src/version.ts +++ b/frontend/src/version.ts @@ -1 +1 @@ -export const APP_VERSION = 'v0.17.5 Beta'; +export const APP_VERSION = 'v0.17.6 Beta';