dns: valid replies fail with EFORMERR when ares_dns_write() fails in QueryWrap::Callback #67043

Version

v24.22.1 (official image); also main at 3983092 (src/cares_wrap.h is unchanged on current main 640ce85)

Platform

Linux x86_64 (Ubuntu 22.04 / Debian, Docker)

Subsystem

dns

What steps will reproduce the bug?

QueryWrap<Traits>::Callback (src/cares_wrap.h:339) re-serializes the record that c-ares already parsed and ignores the result:

    if (status == ARES_SUCCESS) {
      ...
      ares_dns_write(dnsrec, &buf_copy, &answer_len);
    }
    ...
    data->status = status;
    data->buf = MallocedBuffer<unsigned char>(buf_copy, answer_len);

If ares_dns_write() fails, the status stays ARES_SUCCESS with an empty buffer, and parsing that buffer later reports EFORMERR. ares_dns_write() rejects some names that ares_dns_parse() accepted, for example an owner name containing a space, so one such record anywhere in the reply, even in the additional section, makes the whole lookup fail.

A local DNS server that answers example.com A with 192.0.2.1, optionally adding one additional-section A record for x y.example.com:

'use strict';
// node repro.js [clean]
const dgram = require('dgram');
const dns = require('dns');
const clean = process.argv[2] === 'clean';

const name = (s) => Buffer.concat([...s.split('.').map((l) =>
  Buffer.concat([Buffer.from([l.length]), Buffer.from(l, 'latin1')])), Buffer.from([0])]);
const rrA = (owner) => {
  const h = Buffer.alloc(14);
  h.writeUInt16BE(1, 0); h.writeUInt16BE(1, 2); h.writeUInt32BE(300, 4);  // A IN 300
  h.writeUInt16BE(4, 8); h.set([192, 0, 2, 1], 10);
  return Buffer.concat([name(owner), h]);
};

const server = dgram.createSocket('udp4');
server.on('message', (q, rinfo) => {
  let off = 12;
  while (q[off] !== 0) off += q[off] + 1;
  const question = q.subarray(12, off + 5);
  const hdr = Buffer.alloc(12);
  q.copy(hdr, 0, 0, 2);
  hdr.writeUInt16BE(0x8180, 2);                    // response, RD, RA, NOERROR
  hdr.writeUInt16BE(1, 4); hdr.writeUInt16BE(1, 6); hdr.writeUInt16BE(clean ? 0 : 1, 10);
  const additional = clean ? [] : [rrA('x y.example.com')];
  server.send(Buffer.concat([hdr, question, rrA('example.com'), ...additional]),
              rinfo.port, rinfo.address);
});
server.bind(0, '127.0.0.1', () => {
  const r = new dns.Resolver({ timeout: 1000, tries: 1 });
  r.setServers([`127.0.0.1:${server.address().port}`]);
  r.resolve4('example.com', (err, addrs) => {
    console.log(err ? `error ${err.code}` : addrs);
    server.close();
  });
});

How often does it reproduce? Is there a required condition?

Always, when the reply contains a record whose name ares_dns_write() refuses to serialize.

What is the expected behavior? Why is that the expected behavior?

node repro.js prints [ '192.0.2.1' ], as node repro.js clean does: the answer is valid and the extra record is irrelevant to it. If re-serialization does fail, the error from ares_dns_write() should be reported instead of a misleading EFORMERR from parsing an empty buffer.

What do you see instead?

$ node repro.js clean
[ '192.0.2.1' ]
$ node repro.js
error EFORMERR

The same happens with the bad name in the answer section. No memory error under ASan.

Additional information

Checking the return value of ares_dns_write() and passing its status on (instead of ARES_SUCCESS with a NULL buffer) would at least report the real failure; whether the lookup should succeed in this case depends on whether node keeps re-serializing the parsed record.

This issue was found by an LLM-Enhanced Static Analysis Engine and verified manually.