Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CVE-2017-16100: Abandon validation using an unsafe regex pattern in favor of tokenizing and applying a linear pattern #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions lib/dns-sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ var util = require('util'),
shell = require('shelljs'),
debug = require('debug')('dns-sync');

//source - http://stackoverflow.com/questions/106179/regular-expression-to-match-dns-hostname-or-ip-address
var ValidHostnameRegex = new RegExp("^(([a-zA-Z0-9]|[a-zA-Z0-9][a-zA-Z0-9\-]*[a-zA-Z0-9])\.)*([A-Za-z0-9]|[A-Za-z0-9][A-Za-z0-9\-]*[A-Za-z0-9])$");

// https://nodejs.org/api/dns.html#dns_dns_resolve_hostname_rrtype_callback
var RRecordTypes = [
'A',
Expand All @@ -22,8 +19,23 @@ var RRecordTypes = [
'TXT',
'ANY'];

/* RFC 1123: https://datatracker.ietf.org/doc/html/rfc1123
* For a hostname to be valid:
* 1. It needs to have less than 254 chars (255 - 1 for the omitted delimiter of root label)
* 2. Each label should have less than 64 chars but less 63 for right-most label
* 3. Labels are delimited by a period
* 4. Labels start and end with alphanumeric but can have - in middle
*/
const validLabelPattern = /^[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?$/;
function isValidHostName(hostname) {
return ValidHostnameRegex.test(hostname);
if (typeof hostname !== 'string') return false;

let cleanHostname = hostname.endsWith('.') ? hostname.slice(0, -1) : hostname;
if (cleanHostname.length >= 254) return false;

return cleanHostname
.split('.')
.every(label => validLabelPattern.test(label));
}
/**
* Resolve hostname to IP address,
Expand Down
5 changes: 5 additions & 0 deletions test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ describe('dns sync', function () {
assert.ok(dnsSync.resolve('www.paypal.com'));
assert.ok(dnsSync.resolve('www.google.com'));
assert.ok(dnsSync.resolve('www.yahoo.com'));
assert.ok(dnsSync.resolve('0'.repeat(63) + '.000'));
assert.ok(dnsSync.resolve('0'.repeat(63) + '.' + '0'.repeat(63)));
});

it('should fail to resolve dns', function () {
Expand All @@ -24,6 +26,9 @@ describe('dns sync', function () {
assert.ok(!dnsSync.resolve("$(id > /tmp/foo)'"));
assert.ok(!dnsSync.resolve("cd /tmp; rm -f /tmp/echo; env 'x=() { (a)=>' bash -c \"echo date\"; cat /tmp/echo"));
assert.ok(!dnsSync.resolve("$(grep -l -z '[^)]=() {' /proc/[1-9]*/environ | cut -d/ -f3)'"));
assert.ok(!dnsSync.resolve('0-' + '00000'.repeat(29) + '\\n'));
assert.ok(!dnsSync.resolve('0'.repeat(64) + '.000'));
assert.ok(!dnsSync.resolve('0.' + '0'.repeat(64)));
});

it('should resolve AAAA records if asked', function () {
Expand Down