Skip to content

Commit

Permalink
Use dns.lookup over dns.resolve4 in IPTools.dnsblQuery
Browse files Browse the repository at this point in the history 
This helps prevent DNS poisoning attacks if the platform supports DNSSEC
since dns.resolve4 uses c-ares, which doesn't support DNSSEC.
  • Loading branch information
@Kaiepi
Kaiepi committed Jul 9, 2019
1 parent b1891ab commit d4c4796
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion server/ip-tools.ts
View file
Expand Up @@ -190,7 +190,7 @@ export const IPTools = new class {
return;
}
const blocklist = BLOCKLISTS[index];
dns.resolve4(reversedIpDot + blocklist, (err, addresses) => {
dns.lookup(reversedIpDot + blocklist, 4, (err, res) => {
if (!err) {
// blocked
IPTools.dnsblCache.set(ip, blocklist);
Expand Down

3 comments on commit d4c4796

@Slayer95
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this enables an event-loop starvation attack vector prior to Node 10.12.0 nodejs/node#8436

@scheibo
Copy link
Contributor

@scheibo scheibo commented on d4c4796 Jul 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we update our supported version in the README to 10.12.0 then?

@Slayer95
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably.

I am expecting this change to decrease the throughput of this module, though, to a degree which might or might not warrant a reversion, since security of this component is not critical imo.

Please sign in to comment.