From 7671c9d4b2b9b8c4470213a59e39fb2c4ec71d7d Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Mon, 25 May 2026 18:08:19 -0700 Subject: [PATCH] fix: TXT decode preserves string-boundary info --- CHANGELOG.md | 5 ++++ package-lock.json | 4 +-- package.json | 2 +- packet.js | 34 +++++++++++++++++-------- test/packet.js | 64 +++++++++++++++++++++++++++++++++++++++++------ test/server.js | 6 ++--- 6 files changed, 91 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a18369..0df9c06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Unreleased +### [3.0.0] - 2026-05-26 + +- **BREAKING**, TXT `data` is now always an array of strings +- fix(packet): TXT decode preserves character-string boundaries (RFC 1035 §3.3.14) + ### [2.4.0] - 2026-05-26 - feat(ESM): dual published with ESM support diff --git a/package-lock.json b/package-lock.json index 3ad2a7a..4bb41ea 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "dns2", - "version": "2.3.0", + "version": "3.0.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "dns2", - "version": "2.3.0", + "version": "3.0.0", "license": "MIT", "devDependencies": { "@eslint/js": "^10.0.1", diff --git a/package.json b/package.json index 62dba2d..98ea5cb 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "dns2", - "version": "2.4.0", + "version": "3.0.0", "description": "A DNS Server and Client Implementation in Pure JavaScript with no dependencies.", "main": "index.js", "types": "ts/index.d.ts", diff --git a/packet.js b/packet.js index ff6b3d0..9b77f55 100644 --- a/packet.js +++ b/packet.js @@ -768,22 +768,36 @@ Packet.Resource.PTR = Packet.Resource.CNAME = { * @docs https://tools.ietf.org/html/rfc1035#section-3.3.14 */ Packet.Resource.SPF = Packet.Resource.TXT = { + // RFC 1035 §3.3.14: TXT RDATA is one or more length-prefixed + // items. Preserve those boundaries by returning an + // array — joining them silently corrupts SPF/DKIM and other multi-string + // records whose semantics depend on segmentation. decode: function (reader, length) { - const parts = []; + const strings = []; let bytesRead = 0; - let chunkLength; - while (bytesRead < length) { - chunkLength = reader.read(8); // text length + const chunkLength = reader.read(8); bytesRead++; - - while (chunkLength--) { - parts.push(reader.read(8)); - bytesRead++; + // A character-string whose length runs past the end of RDATA would + // make us read into the next record. Skip the remainder of the rdata + // before throwing so the next record decodes from the correct offset + // instead of cascading the error through every following RR. + if (chunkLength > length - bytesRead) { + const remaining = length - bytesRead; + for (let i = 0; i < remaining; i++) reader.read(8); + throw new Error( + `TXT decode: character-string of ${chunkLength} octets overruns ` + + `RDATA (${remaining} octets remaining)`, + ); + } + const bytes = Buffer.alloc(chunkLength); + for (let i = 0; i < chunkLength; i++) { + bytes[i] = reader.read(8); } + bytesRead += chunkLength; + strings.push(bytes.toString('utf8')); } - - this.data = Buffer.from(parts).toString('utf8'); + this.data = strings; return this; }, encode: function (record, writer) { diff --git a/test/packet.js b/test/packet.js index a60bbca..28e0738 100644 --- a/test/packet.js +++ b/test/packet.js @@ -317,7 +317,8 @@ test('Packet#encode', function () { type: Packet.TYPE.TXT, class: Packet.CLASS.IN, ttl: 300, - data: '#v=spf1 include:_spf.google.com ~all', + // TXT data is an array of items (RFC 1035 §3.3.14). + data: ['#v=spf1 include:_spf.google.com ~all'], }); assert.deepEqual(Packet.parse(response.toBuffer()), response); @@ -343,10 +344,7 @@ test('Packet#encode array of character strings', function () { data: dkim, }); - assert.equal( - Packet.parse(response.toBuffer()).answers[0].data, - dkim.join(''), - ); + assert.deepEqual(Packet.parse(response.toBuffer()).answers[0].data, dkim); }); test('EDNS.ECS#encode', function () { @@ -511,9 +509,9 @@ test('Resource#TXT round-trip single string', function () { type: Packet.TYPE.TXT, class: Packet.CLASS.IN, ttl: 300, - data: 'hello world', + data: 'hello world', // encoder normalizes string → [string] }); - assert.equal(out.data, 'hello world'); + assert.deepEqual(out.data, ['hello world']); }); test('Resource#TXT round-trip with utf-8', function () { @@ -524,7 +522,57 @@ test('Resource#TXT round-trip with utf-8', function () { ttl: 300, data: 'café résumé 日本', }); - assert.equal(out.data, 'café résumé 日本'); + assert.deepEqual(out.data, ['café résumé 日本']); +}); + +test('Resource#TXT preserves character-string boundaries', function () { + // SPF/DKIM-style multi-string TXT records must not be merged on decode. + const chunks = ['part-one ', 'part-two ', 'part-three']; + const out = roundTripAnswer({ + name: 'multi.example', + type: Packet.TYPE.TXT, + class: Packet.CLASS.IN, + ttl: 60, + data: chunks, + }); + assert.deepEqual(out.data, chunks); +}); + +test('Resource#TXT decode rejects character-string overruns RDATA', function () { + // Hand-built TXT rdata: rdlength=5, but the first character-string claims + // length 10. Direct caller should see a thrown error. + const reader = new Packet.Reader(Buffer.from([0x0a, 0x61, 0x62, 0x63, 0x64])); + assert.throws( + () => Packet.Resource.TXT.decode.call({}, reader, 5), + /overruns RDATA/, + ); +}); + +test('Resource#TXT decode error does not cascade to following RRs', function () { + // A malformed TXT record (rdlength=5, chunkLength=10) must consume its + // declared rdlength before throwing, so the A record after it parses + // cleanly. Without the bounds check the reader would be misaligned and + // the second record would be lost. + const pkt = Buffer.from([ + // header: id=1, ancount=2, others 0 + 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, + // answer 1: name "t", TYPE=TXT, CLASS=IN, TTL=60, RDLENGTH=5, + // rdata = [chunkLen=10, 4 bogus bytes] ← chunkLen overruns + 0x01, 0x74, 0x00, 0x00, 0x10, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, + 0x05, 0x0a, 0x61, 0x62, 0x63, 0x64, + // answer 2: name "a", TYPE=A, CLASS=IN, TTL=60, RDLENGTH=4, 192.0.2.7 + 0x01, 0x61, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x3c, 0x00, + 0x04, 0xc0, 0x00, 0x02, 0x07, + ]); + const parsed = Packet.parse(pkt); + assert.equal( + parsed.answers.length, + 1, + 'the malformed TXT should be dropped, leaving only the A record', + ); + assert.equal(parsed.answers[0].type, Packet.TYPE.A); + assert.equal(parsed.answers[0].name, 'a'); + assert.equal(parsed.answers[0].address, '192.0.2.7'); }); test('Resource#SOA round-trip', function () { diff --git a/test/server.js b/test/server.js index 96bc60e..1a8257d 100644 --- a/test/server.js +++ b/test/server.js @@ -151,7 +151,7 @@ test('server/udp-tcp#simple-request-async-response', async () => { const tcpClient = TCPClient({ dns: '127.0.0.1', port: servers.tcp.port }); const udpClient = UDPClient({ dns: '127.0.0.1', port: servers.udp.port }); const expected = [ - { name: 'test.com', ttl: 300, type: 16, class: 1, data: 'Hello World' }, + { name: 'test.com', ttl: 300, type: 16, class: 1, data: ['Hello World'] }, ]; assert.deepEqual((await tcpClient('test.com')).answers, expected); assert.deepEqual((await udpClient('test.com')).answers, expected); @@ -456,7 +456,7 @@ test('server/doh#POST end-to-end', async () => { type: Packet.TYPE.TXT, class: Packet.CLASS.IN, ttl: 60, - data: 'post-ok', + data: ['post-ok'], }); send(response); }); @@ -499,7 +499,7 @@ test('server/doh#POST end-to-end', async () => { }); const parsed = Packet.parse(body); assert.equal(parsed.answers.length, 1); - assert.equal(parsed.answers[0].data, 'post-ok'); + assert.deepEqual(parsed.answers[0].data, ['post-ok']); server.close(); });