mafintosh / dns-packet

An abstract-encoding compliant module for encoding / decoding DNS packets
MIT License
205 stars 71 forks source link

Trailing dot in name gets an unfilled byte #62

Open thenewwazoo opened 3 years ago

thenewwazoo commented 3 years ago

When a question's name field has a trailing dot (e.g. 1.0.0.127.in-addr.arpa.), the length is computed before the trailing dot is dropped. This results in a buffer that's one byte longer than necessary, and which has a trailing byte that goes unfilled. For example,

        var reversed = '151.1.168.192.in-addr.arpa.';
        resolver.query({
            flags: 1 << 8 | 1 << 5,
            id: getRandomInt(1, 65534),
            questions: [
                {
                    name: reversed,
                    type: 'PTR',
                    class: 'IN'
                },
            ],
            additionals: [
                {
                    name: '.',
                    type: 'OPT',
                    udpPayloadSize: 0x1000,
                },
            ],
        });

results in a buffer len of 56, but only 55 bytes are written.

The relevant code is here:

name.encode = function (str, buf, offset) {
  if (!buf) buf = Buffer.allocUnsafe(name.encodingLength(str)) // <-- allocate based on str len
  if (!offset) offset = 0
  const oldOffset = offset

  // strip leading and trailing .
  const n = str.replace(/^\.|\.$/gm, '') // <-- mutate str
  if (n.length) {
    const list = n.split('.') // <-- loses the last byte

This was discovered while diagnosing #52 (also trying to diagnose https://github.com/mafintosh/multicast-dns/issues/13).

As a workaround, manually dropping the trailing dot in constructing the address seems to work (but not to fix my issue, oh well.)

mafintosh commented 3 years ago

Ah, I see. Good find. Wanna PR a fix?

thenewwazoo commented 3 years ago

I’ve gotten started but the encodeLength method is called in several places and I need to be confident I’m not going to break anything and that the tests are correct (plus it’s slow going since I’m not very familiar with JS :) ).