Skip to content

Commit

Permalink
dns: fix TTL value for AAAA replies to resolveAny()
Browse files Browse the repository at this point in the history
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: #25187
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
addaleax authored and targos committed Jan 1, 2019
1 parent c13e5be commit 5b4fab1
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 5 deletions.
4 changes: 3 additions & 1 deletion src/cares_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1265,14 +1265,16 @@ 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<Object> obj = Object::New(env()->isolate());
obj->Set(context,
env()->address_string(),
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();
Expand Down
13 changes: 9 additions & 4 deletions test/parallel/test-dns-resolveany.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

0 comments on commit 5b4fab1

Please sign in to comment.