From 4fcfa5a63f4dd50cfe0119e58754ca127aa0e8e3 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 22 Dec 2018 21:18:13 +0100 Subject: [PATCH] dns: fix TTL value for AAAA replies to `resolveAny()` We were previously reading from the wrong offset, namely the one into the final results array, not the one for the AAAA results itself, which could have lead to reading uninitialized or out-of-bounds data. Also, adjust the test accordingly; TTL values are not modified by c-ares, but are only exposed for a subset of all DNS record types. PR-URL: https://github.com/nodejs/node/pull/25187 Reviewed-By: Luigi Pinca Reviewed-By: James M Snell Reviewed-By: Khaidi Chu Reviewed-By: Ben Noordhuis Reviewed-By: Ruben Bridgewater --- src/cares_wrap.cc | 4 +++- test/parallel/test-dns-resolveany.js | 13 +++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index a13ab4b037efca..89a9ee8b79a1a0 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1263,6 +1263,7 @@ class QueryAnyWrap: public QueryWrap { } CHECK_EQ(aaaa_count, naddr6ttls); + CHECK_EQ(ret->Length(), a_count + aaaa_count); for (uint32_t i = a_count; i < ret->Length(); i++) { Local obj = Object::New(env()->isolate()); obj->Set(context, @@ -1270,7 +1271,8 @@ class QueryAnyWrap: public QueryWrap { ret->Get(context, i).ToLocalChecked()).FromJust(); obj->Set(context, env()->ttl_string(), - Integer::New(env()->isolate(), addr6ttls[i].ttl)).FromJust(); + Integer::New(env()->isolate(), addr6ttls[i - a_count].ttl)) + .FromJust(); obj->Set(context, env()->type_string(), env()->dns_aaaa_string()).FromJust(); diff --git a/test/parallel/test-dns-resolveany.js b/test/parallel/test-dns-resolveany.js index 46694f240336cf..bb15b1a38b4ad3 100644 --- a/test/parallel/test-dns-resolveany.js +++ b/test/parallel/test-dns-resolveany.js @@ -53,8 +53,13 @@ server.bind(0, common.mustCall(async () => { })); function validateResults(res) { - // Compare copies with ttl removed, c-ares fiddles with that value. - assert.deepStrictEqual( - res.map((r) => Object.assign({}, r, { ttl: null })), - answers.map((r) => Object.assign({}, r, { ttl: null }))); + // TTL values are only provided for A and AAAA entries. + assert.deepStrictEqual(res.map(maybeRedactTTL), answers.map(maybeRedactTTL)); +} + +function maybeRedactTTL(r) { + const ret = { ...r }; + if (!['A', 'AAAA'].includes(r.type)) + delete ret.ttl; + return ret; }