Skip to content

Commit

Permalink
fix(NODE-5042): relax SRV record validation to account for a dot suff…
Browse files Browse the repository at this point in the history
…ix (#3633)
  • Loading branch information
nbbeeken authored and baileympearson committed Apr 13, 2023
1 parent c11e2cf commit 089f745
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 31 deletions.
16 changes: 1 addition & 15 deletions src/connection_string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
HostAddress,
isRecord,
makeClientMetadata,
matchesParentDomain,
parseInteger,
setDifference
} from './utils';
Expand All @@ -47,21 +48,6 @@ const LB_REPLICA_SET_ERROR = 'loadBalanced option not supported with a replicaSe
const LB_DIRECT_CONNECTION_ERROR =
'loadBalanced option not supported when directConnection is provided';

/**
* Determines whether a provided address matches the provided parent domain in order
* to avoid certain attack vectors.
*
* @param srvAddress - The address to check against a domain
* @param parentDomain - The domain to check the provided address against
* @returns Whether the provided address matches the parent domain
*/
function matchesParentDomain(srvAddress: string, parentDomain: string): boolean {
const regex = /^.*?\./;
const srv = `.${srvAddress.replace(regex, '')}`;
const parent = `.${parentDomain.replace(regex, '')}`;
return srv.endsWith(parent);
}

/**
* Lookup a `mongodb+srv` connection string, combine the parts and reparse it as a normal
* connection string.
Expand Down
17 changes: 1 addition & 16 deletions src/sdam/srv_polling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,7 @@ import { clearTimeout, setTimeout } from 'timers';
import { MongoRuntimeError } from '../error';
import { Logger, LoggerOptions } from '../logger';
import { TypedEventEmitter } from '../mongo_types';
import { HostAddress } from '../utils';

/**
* Determines whether a provided address matches the provided parent domain in order
* to avoid certain attack vectors.
*
* @param srvAddress - The address to check against a domain
* @param parentDomain - The domain to check the provided address against
* @returns Whether the provided address matches the parent domain
*/
function matchesParentDomain(srvAddress: string, parentDomain: string): boolean {
const regex = /^.*?\./;
const srv = `.${srvAddress.replace(regex, '')}`;
const parent = `.${parentDomain.replace(regex, '')}`;
return srv.endsWith(parent);
}
import { HostAddress, matchesParentDomain } from '../utils';

/**
* @internal
Expand Down
26 changes: 26 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1430,3 +1430,29 @@ export function parseUnsignedInteger(value: unknown): number | null {

return parsedInt != null && parsedInt >= 0 ? parsedInt : null;
}

/**
* Determines whether a provided address matches the provided parent domain.
*
* If a DNS server were to become compromised SRV records would still need to
* advertise addresses that are under the same domain as the srvHost.
*
* @param address - The address to check against a domain
* @param srvHost - The domain to check the provided address against
* @returns Whether the provided address matches the parent domain
*/
export function matchesParentDomain(address: string, srvHost: string): boolean {
// Remove trailing dot if exists on either the resolved address or the srv hostname
const normalizedAddress = address.endsWith('.') ? address.slice(0, address.length - 1) : address;
const normalizedSrvHost = srvHost.endsWith('.') ? srvHost.slice(0, srvHost.length - 1) : srvHost;

const allCharacterBeforeFirstDot = /^.*?\./;
// Remove all characters before first dot
// Add leading dot back to string so
// an srvHostDomain = '.trusted.site'
// will not satisfy an addressDomain that endsWith '.fake-trusted.site'
const addressDomain = `.${normalizedAddress.replace(allCharacterBeforeFirstDot, '')}`;
const srvHostDomain = `.${normalizedSrvHost.replace(allCharacterBeforeFirstDot, '')}`;

return addressDomain.endsWith(srvHostDomain);
}
43 changes: 43 additions & 0 deletions test/unit/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
HostAddress,
isHello,
List,
matchesParentDomain,
maybeCallback,
MongoDBNamespace,
shuffle
Expand Down Expand Up @@ -858,4 +859,46 @@ describe('driver utils', function () {
it(title, () => expect(compareObjectId(oid1, oid2)).to.equal(result));
}
});

describe('matchesParentDomain()', () => {
const exampleSrvName = 'i-love-javascript.mongodb.io';
const exampleSrvNameWithDot = 'i-love-javascript.mongodb.io.';
const exampleHostNameWithoutDot = 'i-love-javascript-00.mongodb.io';
const exampleHostNamesWithDot = exampleHostNameWithoutDot + '.';
const exampleHostNamThatDoNotMatchParent = 'i-love-javascript-00.evil-mongodb.io';
const exampleHostNamThatDoNotMatchParentWithDot = 'i-love-javascript-00.evil-mongodb.io.';

context('when address does not match parent domain', () => {
it('without a trailing dot returns false', () => {
expect(matchesParentDomain(exampleHostNamThatDoNotMatchParent, exampleSrvName)).to.be.false;
});

it('with a trailing dot returns false', () => {
expect(matchesParentDomain(exampleHostNamThatDoNotMatchParentWithDot, exampleSrvName)).to.be
.false;
});
});

context('when addresses in SRV record end with a dot', () => {
it('accepts address since it is considered to still match the parent domain', () => {
expect(matchesParentDomain(exampleHostNamesWithDot, exampleSrvName)).to.be.true;
});
});

context('when SRV host ends with a dot', () => {
it('accepts address if it ends with a dot', () => {
expect(matchesParentDomain(exampleHostNamesWithDot, exampleSrvNameWithDot)).to.be.true;
});

it('accepts address if it does not end with a dot', () => {
expect(matchesParentDomain(exampleHostNameWithoutDot, exampleSrvName)).to.be.true;
});
});

context('when addresses in SRV record end without dots', () => {
it('accepts address since it matches the parent domain', () => {
expect(matchesParentDomain(exampleHostNamesWithDot, exampleSrvName)).to.be.true;
});
});
});
});

0 comments on commit 089f745

Please sign in to comment.