Skip to content

Commit

Permalink
Make name.decode stricter (#79)
Browse files Browse the repository at this point in the history
* Add tests for name decoding corner cases

* Modify name.decode to throw an error in the following cases:
 * Not enough data for reading the full label
 * The label is too long (over 253 characters when dots are included)
  * A label must be either <= 63 bytes or a pointer
  * Pointers can only point to prior data (see RFC 1035, section 4.1.4)

In addition pointer jumps don't add extra dots in the names anymore.

* Make name_decoding tests more specific

* Make name.decode non-recursive

* Ensure name.decode can read the label header

* Fix name.decode error messages
  • Loading branch information
jviide authored Dec 23, 2021
1 parent 8e6d91c commit 1d42aad
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 22 deletions.
64 changes: 42 additions & 22 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,33 +46,53 @@ name.decode = function (buf, offset) {
if (!offset) offset = 0

const list = []
const oldOffset = offset
let len = buf[offset++]

if (len === 0) {
name.decode.bytes = 1
return '.'
}
if (len >= 0xc0) {
const res = name.decode(buf, buf.readUInt16BE(offset - 1) - 0xc000)
name.decode.bytes = 2
return res
}
let oldOffset = offset
let totalLength = 0
let consumedBytes = 0
let jumped = false

while (true) {
if (offset >= buf.length) {
throw new Error('Cannot decode name (buffer overflow)')
}
const len = buf[offset++]
consumedBytes += jumped ? 0 : 1

while (len) {
if (len >= 0xc0) {
list.push(name.decode(buf, buf.readUInt16BE(offset - 1) - 0xc000))
offset++
if (len === 0) {
break
} else if ((len & 0xc0) === 0) {
if (offset + len > buf.length) {
throw new Error('Cannot decode name (buffer overflow)')
}
totalLength += len + 1
if (totalLength > 254) {
throw new Error('Cannot decode name (name too long)')
}
list.push(buf.toString('utf-8', offset, offset + len))
offset += len
consumedBytes += jumped ? 0 : len
} else if ((len & 0xc0) === 0xc0) {
if (offset + 1 > buf.length) {
throw new Error('Cannot decode name (buffer overflow)')
}
const jumpOffset = buf.readUInt16BE(offset - 1) - 0xc000
if (jumpOffset >= oldOffset) {
// Allow only pointers to prior data. RFC 1035, section 4.1.4 states:
// "[...] an entire domain name or a list of labels at the end of a domain name
// is replaced with a pointer to a prior occurance (sic) of the same name."
throw new Error('Cannot decode name (bad pointer)')
}
offset = jumpOffset
oldOffset = jumpOffset
consumedBytes += jumped ? 0 : 1
jumped = true
} else {
throw new Error('Cannot decode name (bad label)')
}

list.push(buf.toString('utf-8', offset, offset + len))
offset += len
len = buf[offset++]
}

name.decode.bytes = offset - oldOffset
return list.join('.')
name.decode.bytes = consumedBytes
return list.length === 0 ? '.' : list.join('.')
}

name.decode.bytes = 0
Expand Down
44 changes: 44 additions & 0 deletions test.js
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,50 @@ tape('name_encoding', function (t) {
t.end()
})

tape('name_decoding', function (t) {
// The two most significant bits of a valid label header must be either both zero or both one
t.throws(function () { packet.name.decode(Buffer.from([0x80])) }, /Cannot decode name \(bad label\)$/)
t.throws(function () { packet.name.decode(Buffer.from([0xb0])) }, /Cannot decode name \(bad label\)$/)

// Ensure there's enough buffer to read
t.throws(function () { packet.name.decode(Buffer.from([])) }, /Cannot decode name \(buffer overflow\)$/)
t.throws(function () { packet.name.decode(Buffer.from([0x01, 0x00])) }, /Cannot decode name \(buffer overflow\)$/)
t.throws(function () { packet.name.decode(Buffer.from([0x01])) }, /Cannot decode name \(buffer overflow\)$/)
t.throws(function () { packet.name.decode(Buffer.from([0xc0])) }, /Cannot decode name \(buffer overflow\)$/)

// Allow only pointers backwards
t.throws(function () { packet.name.decode(Buffer.from([0xc0, 0x00])) }, /Cannot decode name \(bad pointer\)$/)
t.throws(function () { packet.name.decode(Buffer.from([0xc0, 0x01])) }, /Cannot decode name \(bad pointer\)$/)

// A name can be only 253 characters (when connected with dots)
const maxLength = Buffer.alloc(255)
maxLength.fill(Buffer.from([0x01, 0x61]), 0, 254)
t.ok(packet.name.decode(maxLength) === new Array(127).fill('a').join('.'))

const tooLong = Buffer.alloc(256)
tooLong.fill(Buffer.from([0x01, 0x61]))
t.throws(function () { packet.name.decode(tooLong) }, /Cannot decode name \(name too long\)$/)

// Ensure jumps don't reset the total length counter
const tooLongWithJump = Buffer.alloc(403)
tooLongWithJump.fill(Buffer.from([0x01, 0x61]), 0, 200)
tooLongWithJump.fill(Buffer.from([0x01, 0x61]), 201, 401)
tooLongWithJump.set([0xc0, 0x00], 401)
t.throws(function () { packet.name.decode(tooLongWithJump, 201) }, /Cannot decode name \(name too long\)$/)

// Ensure a jump to a null byte doesn't add extra dots
t.ok(packet.name.decode(Buffer.from([0x00, 0x01, 0x61, 0xc0, 0x00]), 1) === 'a')

// Ensure deeply nested pointers don't cause "Maximum call stack size exceeded" errors
const buf = Buffer.alloc(16386)
for (let i = 0; i < 16384; i += 2) {
buf.writeUInt16BE(0xc000 | i, i + 2)
}
t.ok(packet.name.decode(buf, 16384) === '.')

t.end()
})

tape('stream', function (t) {
const val = {
type: 'query',
Expand Down

0 comments on commit 1d42aad

Please sign in to comment.