dns.resolveSoa returns EBADRESP if hostname has a CNAME record

[jlchristi]

• Version(s): v12.16.3 / v12.18.3 / v12.18.0
• Platform: Windows / Windows Subsystem for Linux (Debian) / Red Hat Enterprise Linux Server release 7.8
• Subsystem: DNS

What steps will reproduce the bug?

const dns = require('dns');
const resolver = new dns.promises.Resolver();

// **************************************************************

async function test(hostname) {
  let data = '';

  console.log('hostname:', hostname);

  try {
    data = await resolver.resolveCname(hostname);
    console.log('CNAME result:', data);
  } catch (error) {
    console.log('CNAME result:', error.message);
  }

  try {
    data = await resolver.resolveSoa(hostname);
    console.log('SOA result:', JSON.stringify(data));
  } catch (error) {
    console.log('SOA result:', error.message);
  }

  console.log();
}

// **************************************************************

(async function main() {
  await test('support.microsoft.com');
})();

Actual Results

hostname: support.microsoft.com
CNAME result: [ 'ev.support.microsoft.com.edgekey.net' ]
SOA result: querySoa EBADRESP support.microsoft.com

Expected Results

hostname: support.microsoft.com
CNAME result: [ 'ev.support.microsoft.com.edgekey.net' ]
SOA result: querySoa ENODATA support.microsoft.com

Additional information

This seems to happen for any hostname with a CNAME record.

Another example:

hostname: store.gocomics.com
CNAME result: [ 'gocomicsstore.wpengine.com' ]
SOA result: querySoa EBADRESP store.gocomics.com

I would expect to get an 'ENODATA' instead of 'EBADRESP', as with the other resolveXXX() calls.

For a hostname with an SOA record but no CNAME, you get:

hostname: microsoft.com
CNAME result: queryCname ENODATA microsoft.com
SOA result: {"nsname":"ns1-205.azure-dns.com","hostmaster":"azuredns-
hostmaster.microsoft.com","serial":1,"refresh":3600,"retry":300,"expire":2419200,"minttl":300}

[bnoordhuis]

Thanks for the bug report. At first glance this appears to be a bug in our response parsing logic.

---

node/src/cares_wrap.cc

Lines 1078 to 1080 in 74df749

	if (ptr + temp_len + NS_QFIXEDSZ > buf + len) {
	return ARES_EBADRESP;
	}

@addaleax @XadillaX Maybe not the root cause but those bounds checks look like UB to me.

Pointers in C and C++ are allowed to point one element past the end of an array and no more. The check should look like this:

diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc
index 73a0ac6b33..778e0821d7 100644
--- a/src/cares_wrap.cc
+++ b/src/cares_wrap.cc
@@ -1075,7 +1075,7 @@ int ParseSoaReply(Environment* env,
     return status == ARES_EBADNAME ? ARES_EBADRESP : status;
   }
 
-  if (ptr + temp_len + NS_QFIXEDSZ > buf + len) {
+  if (temp_len > LONG_MAX - NS_QFIXEDSZ || temp_len + NS_QFIXEDSZ > len) {
     return ARES_EBADRESP;
   }
   ptr += temp_len + NS_QFIXEDSZ;

[AasthaGupta]

@bnoordhuis I'd like to work on this.

[AasthaGupta]

@bnoordhuis -
I tried tracing down function calls and I don't think the problem exists in parseSoaReply. Instead, the Parse function of QuerySoaWrap is invoked.

This Parse function handles the parsing when the ares_query invokes its callback with a success status.
In the case when SOA response doesn't exist, the ares_query function invokes the callback with a success status instead of ENODATA.

At this point, I think the problem exists in the ares_query library function itself.

Also while working on this, I discovered a bug related to free call on garbage pointer. #35502

[nordluf]

v20.11.0 the bug is still there