Skip to content

dns.resolveSoa returns EBADRESP if hostname has a CNAME record #34612

Description

@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}

Activity

  1. added
    confirmed-bugIssues and PRs for confirmed bugs.
    dnsIssues and PRs related to the dns subsystem.
    on Aug 4, 2020
  2. bnoordhuis commented on Aug 4, 2020

    @bnoordhuis
    Member

    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;
  3. AasthaGupta commented on Oct 3, 2020

    @AasthaGupta
    Contributor

    @bnoordhuis I'd like to work on this.

  4. AasthaGupta commented on Oct 4, 2020

    @AasthaGupta
    Contributor

    @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

  5. nordluf commented on Jan 27, 2024

    @nordluf

    v20.11.0 the bug is still there

  6. santigimeno commented on Jan 27, 2025

    @santigimeno
    Member

    This issue (if so) comes indeed from this condition in cares and should be reported upstream in case they want to change it. Should we keep this open or just close it?

  7. added
    caresIssues and PRs related to the c-ares dependency or the cares_wrap binding.
    on Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    caresIssues and PRs related to the c-ares dependency or the cares_wrap binding.confirmed-bugIssues and PRs for confirmed bugs.dnsIssues and PRs related to the dns subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions