From 089f7450dcccbe236e1f62f9706c7603a7017f53 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 12 Apr 2023 07:02:53 -0400 Subject: [PATCH] fix(NODE-5042): relax SRV record validation to account for a dot suffix (#3633) --- src/connection_string.ts | 16 +-------------- src/sdam/srv_polling.ts | 17 +--------------- src/utils.ts | 26 ++++++++++++++++++++++++ test/unit/utils.test.ts | 43 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 31 deletions(-) diff --git a/src/connection_string.ts b/src/connection_string.ts index 3abbcf9228..4343f055ba 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -35,6 +35,7 @@ import { HostAddress, isRecord, makeClientMetadata, + matchesParentDomain, parseInteger, setDifference } from './utils'; @@ -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. diff --git a/src/sdam/srv_polling.ts b/src/sdam/srv_polling.ts index f9fc60193e..7202282c1e 100644 --- a/src/sdam/srv_polling.ts +++ b/src/sdam/srv_polling.ts @@ -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 diff --git a/src/utils.ts b/src/utils.ts index 57e2ec3566..9e9c20bdf7 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -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); +} diff --git a/test/unit/utils.test.ts b/test/unit/utils.test.ts index fccfa52473..c23b717cca 100644 --- a/test/unit/utils.test.ts +++ b/test/unit/utils.test.ts @@ -11,6 +11,7 @@ import { HostAddress, isHello, List, + matchesParentDomain, maybeCallback, MongoDBNamespace, shuffle @@ -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; + }); + }); + }); });