Skip to content

Commit ad15881

Browse files
authored
fix(NODE-5042): relax SRV record validation to account for a dot suffix (#3633)
1 parent 8a7ce1f commit ad15881

File tree

4 files changed

+71
-31
lines changed

4 files changed

+71
-31
lines changed

src/connection_string.ts

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
HostAddress,
3434
isRecord,
3535
makeClientMetadata,
36+
matchesParentDomain,
3637
parseInteger,
3738
setDifference
3839
} from './utils';
@@ -45,21 +46,6 @@ const LB_REPLICA_SET_ERROR = 'loadBalanced option not supported with a replicaSe
4546
const LB_DIRECT_CONNECTION_ERROR =
4647
'loadBalanced option not supported when directConnection is provided';
4748

48-
/**
49-
* Determines whether a provided address matches the provided parent domain in order
50-
* to avoid certain attack vectors.
51-
*
52-
* @param srvAddress - The address to check against a domain
53-
* @param parentDomain - The domain to check the provided address against
54-
* @returns Whether the provided address matches the parent domain
55-
*/
56-
function matchesParentDomain(srvAddress: string, parentDomain: string): boolean {
57-
const regex = /^.*?\./;
58-
const srv = `.${srvAddress.replace(regex, '')}`;
59-
const parent = `.${parentDomain.replace(regex, '')}`;
60-
return srv.endsWith(parent);
61-
}
62-
6349
/**
6450
* Lookup a `mongodb+srv` connection string, combine the parts and reparse it as a normal
6551
* connection string.

src/sdam/srv_polling.ts

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,7 @@ import { clearTimeout, setTimeout } from 'timers';
33

44
import { MongoRuntimeError } from '../error';
55
import { TypedEventEmitter } from '../mongo_types';
6-
import { HostAddress } from '../utils';
7-
8-
/**
9-
* Determines whether a provided address matches the provided parent domain in order
10-
* to avoid certain attack vectors.
11-
*
12-
* @param srvAddress - The address to check against a domain
13-
* @param parentDomain - The domain to check the provided address against
14-
* @returns Whether the provided address matches the parent domain
15-
*/
16-
function matchesParentDomain(srvAddress: string, parentDomain: string): boolean {
17-
const regex = /^.*?\./;
18-
const srv = `.${srvAddress.replace(regex, '')}`;
19-
const parent = `.${parentDomain.replace(regex, '')}`;
20-
return srv.endsWith(parent);
21-
}
6+
import { HostAddress, matchesParentDomain } from '../utils';
227

238
/**
249
* @internal

src/utils.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1277,3 +1277,29 @@ export function parseUnsignedInteger(value: unknown): number | null {
12771277

12781278
return parsedInt != null && parsedInt >= 0 ? parsedInt : null;
12791279
}
1280+
1281+
/**
1282+
* Determines whether a provided address matches the provided parent domain.
1283+
*
1284+
* If a DNS server were to become compromised SRV records would still need to
1285+
* advertise addresses that are under the same domain as the srvHost.
1286+
*
1287+
* @param address - The address to check against a domain
1288+
* @param srvHost - The domain to check the provided address against
1289+
* @returns Whether the provided address matches the parent domain
1290+
*/
1291+
export function matchesParentDomain(address: string, srvHost: string): boolean {
1292+
// Remove trailing dot if exists on either the resolved address or the srv hostname
1293+
const normalizedAddress = address.endsWith('.') ? address.slice(0, address.length - 1) : address;
1294+
const normalizedSrvHost = srvHost.endsWith('.') ? srvHost.slice(0, srvHost.length - 1) : srvHost;
1295+
1296+
const allCharacterBeforeFirstDot = /^.*?\./;
1297+
// Remove all characters before first dot
1298+
// Add leading dot back to string so
1299+
// an srvHostDomain = '.trusted.site'
1300+
// will not satisfy an addressDomain that endsWith '.fake-trusted.site'
1301+
const addressDomain = `.${normalizedAddress.replace(allCharacterBeforeFirstDot, '')}`;
1302+
const srvHostDomain = `.${normalizedSrvHost.replace(allCharacterBeforeFirstDot, '')}`;
1303+
1304+
return addressDomain.endsWith(srvHostDomain);
1305+
}

test/unit/utils.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
isHello,
1010
LEGACY_HELLO_COMMAND,
1111
List,
12+
matchesParentDomain,
1213
maybeCallback,
1314
MongoDBNamespace,
1415
MongoRuntimeError,
@@ -899,4 +900,46 @@ describe('driver utils', function () {
899900
});
900901
});
901902
});
903+
904+
describe('matchesParentDomain()', () => {
905+
const exampleSrvName = 'i-love-javascript.mongodb.io';
906+
const exampleSrvNameWithDot = 'i-love-javascript.mongodb.io.';
907+
const exampleHostNameWithoutDot = 'i-love-javascript-00.mongodb.io';
908+
const exampleHostNamesWithDot = exampleHostNameWithoutDot + '.';
909+
const exampleHostNamThatDoNotMatchParent = 'i-love-javascript-00.evil-mongodb.io';
910+
const exampleHostNamThatDoNotMatchParentWithDot = 'i-love-javascript-00.evil-mongodb.io.';
911+
912+
context('when address does not match parent domain', () => {
913+
it('without a trailing dot returns false', () => {
914+
expect(matchesParentDomain(exampleHostNamThatDoNotMatchParent, exampleSrvName)).to.be.false;
915+
});
916+
917+
it('with a trailing dot returns false', () => {
918+
expect(matchesParentDomain(exampleHostNamThatDoNotMatchParentWithDot, exampleSrvName)).to.be
919+
.false;
920+
});
921+
});
922+
923+
context('when addresses in SRV record end with a dot', () => {
924+
it('accepts address since it is considered to still match the parent domain', () => {
925+
expect(matchesParentDomain(exampleHostNamesWithDot, exampleSrvName)).to.be.true;
926+
});
927+
});
928+
929+
context('when SRV host ends with a dot', () => {
930+
it('accepts address if it ends with a dot', () => {
931+
expect(matchesParentDomain(exampleHostNamesWithDot, exampleSrvNameWithDot)).to.be.true;
932+
});
933+
934+
it('accepts address if it does not end with a dot', () => {
935+
expect(matchesParentDomain(exampleHostNameWithoutDot, exampleSrvName)).to.be.true;
936+
});
937+
});
938+
939+
context('when addresses in SRV record end without dots', () => {
940+
it('accepts address since it matches the parent domain', () => {
941+
expect(matchesParentDomain(exampleHostNamesWithDot, exampleSrvName)).to.be.true;
942+
});
943+
});
944+
});
902945
});

0 commit comments

Comments
 (0)