CVE-2024-39699: Server-Side Request Forgery (SSRF) (CWE-918)
value: Directus is a real-time API and App dashboard for managing SQL database content. There was already a reported SSRF vulnerability via file import. It was fixed by resolving all DNS names and checking if the requested IP is an internal IP address. However it is possible to bypass this security measure and execute a SSRF using redirects. Directus allows redirects when importing file from the URL and does not check the result URL. Thus, it is possible to execute a request to an internal IP, for example to 127.0.0.1. However, it is blind SSRF, because Directus also uses response interception technique to get the information about the connect from the socket directly and it does not show a response if the IP address is internal. This vulnerability is fixed in 10.9.3.
CWE description: The web server receives a URL or similar request from an upstream component and retrieves the contents of this URL, but it does not sufficiently ensure that the request is being sent to the expected destination.
CVSS v3 base score: 5
Published: 2024-07-08T16:15Z
Repository: https://github.com/directus/directus
Language: TypeScript
Files changed: .changeset/dull-comics-camp.md, api/src/services/roles.test.ts, api/src/services/roles.ts
Commit date: 2024-02-19 23:19:55 +0100
Commit message: Add validation for IP Access in Role settings (#21444)


Diff: diff --git a/.changeset/dull-comics-camp.md b/.changeset/dull-comics-camp.md
new file mode 100644
index 00000000000..2f9a9eca835
--- /dev/null
+++ b/.changeset/dull-comics-camp.md
@@ -0,0 +1,5 @@
+---
+'@directus/api': patch
+---
+
+Added validation for IP Access in role settings
diff --git a/api/src/services/roles.test.ts b/api/src/services/roles.test.ts
index 5c020ea65f7..94b18f24815 100644
--- a/api/src/services/roles.test.ts
+++ b/api/src/services/roles.test.ts
@@ -1,20 +1,20 @@
 import { ForbiddenError, UnprocessableContentError } from '@directus/errors';
 import type { SchemaOverview } from '@directus/types';
 import type { Knex } from 'knex';
 import knex from 'knex';
-import { createTracker, MockClient, Tracker, type RawQuery } from 'knex-mock-client';
+import { MockClient, Tracker, createTracker, type RawQuery } from 'knex-mock-client';
 import {
 	afterEach,
 	beforeAll,
 	beforeEach,
 	describe,
 	expect,
 	it,
 	vi,
-	type MockedFunction,
 	type MockInstance,
+	type MockedFunction,
 } from 'vitest';
 
 import { ItemsService, PermissionsService, PresetsService, RolesService, UsersService } from './index.js';
 
 vi.mock('../../src/database/index', () => {
@@ -695,17 +695,29 @@ describe('Integration Tests', () => {
 		describe('createOne', () => {
 			it('should not checkForOtherAdminRoles', async () => {
 				await service.createOne({});
 				expect(checkForOtherAdminRolesSpy).not.toBeCalled();
 			});
+
+			it('should throw due to invalid ip_access', async () => {
+				await expect(service.createOne({ ip_access: ['invalid_ip'] })).rejects.toThrow(
+					'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
+				);
+			});
 		});
 
 		describe('createMany', () => {
 			it('should not checkForOtherAdminRoles', async () => {
 				await service.createMany([{}]);
 				expect(checkForOtherAdminRolesSpy).not.toBeCalled();
 			});
+
+			it('should throw due to invalid ip_access', async () => {
+				await expect(service.createMany([{ ip_access: ['invalid_ip'] }])).rejects.toThrow(
+					'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
+				);
+			});
 		});
 
 		describe('updateOne', () => {
 			it('should not checkForOtherAdminRoles', async () => {
 				await service.updateOne(1, {});
@@ -721,10 +733,16 @@ describe('Integration Tests', () => {
 			it('should checkForOtherAdminRoles and checkForOtherAdminUsersSpy once', async () => {
 				await service.updateOne(1, { admin_access: false, users: [1] });
 				expect(checkForOtherAdminRolesSpy).toBeCalledTimes(1);
 				expect(checkForOtherAdminUsersSpy).toBeCalledTimes(1);
 			});
+
+			it('should throw due to invalid ip_access', async () => {
+				await expect(service.updateOne(1, { ip_access: ['invalid_ip'] })).rejects.toThrow(
+					'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
+				);
+			});
 		});
 
 		describe('updateMany', () => {
 			it('should not checkForOtherAdminRoles', async () => {
 				await service.updateMany([1], {});
@@ -733,10 +751,16 @@ describe('Integration Tests', () => {
 
 			it('should checkForOtherAdminRoles once', async () => {
 				await service.updateMany([1], { admin_access: false });
 				expect(checkForOtherAdminRolesSpy).toBeCalledTimes(1);
 			});
+
+			it('should throw due to invalid ip_access', async () => {
+				await expect(service.updateMany([1], { ip_access: ['invalid_ip'] })).rejects.toThrow(
+					'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
+				);
+			});
 		});
 
 		describe('updateBatch', () => {
 			it('should not checkForOtherAdminRoles', async () => {
 				await service.updateBatch([{ id: 1 }]);
@@ -745,10 +769,16 @@ describe('Integration Tests', () => {
 
 			it('should checkForOtherAdminRoles once', async () => {
 				await service.updateBatch([{ id: 1, admin_access: false }]);
 				expect(checkForOtherAdminRolesSpy).toBeCalledTimes(1);
 			});
+
+			it('should throw due to invalid ip_access', async () => {
+				await expect(service.updateBatch([{ id: 1, ip_access: ['invalid_ip'] }])).rejects.toThrow(
+					'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
+				);
+			});
 		});
 
 		describe('updateByQuery', () => {
 			it('should not checkForOtherAdminRoles', async () => {
 				// mock return value for the following empty query
@@ -761,10 +791,16 @@ describe('Integration Tests', () => {
 				// mock return value for the following empty query
 				vi.spyOn(ItemsService.prototype, 'getKeysByQuery').mockResolvedValueOnce([1]);
 				await service.updateByQuery({}, { admin_access: false });
 				expect(checkForOtherAdminRolesSpy).toBeCalledTimes(1);
 			});
+
+			it('should throw due to invalid ip_access', async () => {
+				await expect(service.updateByQuery({}, { ip_access: ['invalid_ip'] })).rejects.toThrow(
+					'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
+				);
+			});
 		});
 
 		describe('deleteOne', () => {
 			it('should checkForOtherAdminRoles once', async () => {
 				await service.deleteOne(1);
diff --git a/api/src/services/roles.ts b/api/src/services/roles.ts
index c44e344e0f1..1bb0900ea77 100644
--- a/api/src/services/roles.ts
+++ b/api/src/services/roles.ts
@@ -1,8 +1,9 @@
-import { ForbiddenError, UnprocessableContentError } from '@directus/errors';
+import { ForbiddenError, InvalidPayloadError, UnprocessableContentError } from '@directus/errors';
 import type { Query, User } from '@directus/types';
-import type { AbstractServiceOptions, Alterations, MutationOptions, PrimaryKey } from '../types/index.js';
+import { getMatch } from 'ip-matching';
+import type { AbstractServiceOptions, Alterations, Item, MutationOptions, PrimaryKey } from '../types/index.js';
 import { ItemsService } from './items.js';
 import { PermissionsService } from './permissions.js';
 import { PresetsService } from './presets.js';
 import { UsersService } from './users.js';
 
@@ -147,11 +148,54 @@ export class RolesService extends ItemsService {
 		}
 
 		return;
 	}
 
-	override async updateOne(key: PrimaryKey, data: Record<string, any>, opts?: MutationOptions): Promise<PrimaryKey> {
+	private isIpAccessValid(value?: any[] | null): boolean {
+		if (value === undefined) return false;
+		if (value === null) return true;
+		if (Array.isArray(value) && value.length === 0) return true;
+
+		for (const ip of value) {
+			if (typeof ip !== 'string' || ip.includes('*')) return false;
+
+			try {
+				const match = getMatch(ip);
+				if (match.type == 'IPMask') return false;
+			} catch {
+				return false;
+			}
+		}
+
+		return true;
+	}
+
+	private assertValidIpAccess(partialItem: Partial<Item>): void {
+		if ('ip_access' in partialItem && !this.isIpAccessValid(partialItem['ip_access'])) {
+			throw new InvalidPayloadError({
+				reason: 'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
+			});
+		}
+	}
+
+	override async createOne(data: Partial<Item>, opts?: MutationOptions): Promise<PrimaryKey> {
+		this.assertValidIpAccess(data);
+
+		return super.createOne(data, opts);
+	}
+
+	override async createMany(data: Partial<Item>[], opts?: MutationOptions): Promise<PrimaryKey[]> {
+		for (const partialItem of data) {
+			this.assertValidIpAccess(partialItem);
+		}
+
+		return super.createMany(data, opts);
+	}
+
+	override async updateOne(key: PrimaryKey, data: Partial<Item>, opts?: MutationOptions): Promise<PrimaryKey> {
+		this.assertValidIpAccess(data);
+
 		try {
 			if ('users' in data) {
 				await this.checkForOtherAdminUsers(key, data['users']);
 			}
 		} catch (err: any) {
@@ -159,13 +203,16 @@ export class RolesService extends ItemsService {
 		}
 
 		return super.updateOne(key, data, opts);
 	}
 
-	override async updateBatch(data: Record<string, any>[], opts?: MutationOptions): Promise<PrimaryKey[]> {
-		const primaryKeyField = this.schema.collections[this.collection]!.primary;
+	override async updateBatch(data: Partial<Item>[], opts?: MutationOptions): Promise<PrimaryKey[]> {
+		for (const partialItem of data) {
+			this.assertValidIpAccess(partialItem);
+		}
 
+		const primaryKeyField = this.schema.collections[this.collection]!.primary;
 		const keys = data.map((item) => item[primaryKeyField]);
 		const setsToNoAdmin = data.some((item) => item['admin_access'] === false);
 
 		try {
 			if (setsToNoAdmin) {
@@ -176,15 +223,13 @@ export class RolesService extends ItemsService {
 		}
 
 		return super.updateBatch(data, opts);
 	}
 
-	override async updateMany(
-		keys: PrimaryKey[],
-		data: Record<string, any>,
-		opts?: MutationOptions,
-	): Promise<PrimaryKey[]> {
+	override async updateMany(keys: PrimaryKey[], data: Partial<Item>, opts?: MutationOptions): Promise<PrimaryKey[]> {
+		this.assertValidIpAccess(data);
+
 		try {
 			if ('admin_access' in data && data['admin_access'] === false) {
 				await this.checkForOtherAdminRoles(keys);
 			}
 		} catch (err: any) {
@@ -192,10 +237,20 @@ export class RolesService extends ItemsService {
 		}
 
 		return super.updateMany(keys, data, opts);
 	}
 
+	override async updateByQuery(
+		query: Query,
+		data: Partial<Item>,
+		opts?: MutationOptions | undefined,
+	): Promise<PrimaryKey[]> {
+		this.assertValidIpAccess(data);
+
+		return super.updateByQuery(query, data, opts);
+	}
+
 	override async deleteOne(key: PrimaryKey): Promise<PrimaryKey> {
 		await this.deleteMany([key]);
 		return key;
 	}
 

Vulnerable code: import { createTracker, MockClient, Tracker, type RawQuery } from 'knex-mock-client';
	type MockedFunction,
import { ForbiddenError, UnprocessableContentError } from '@directus/errors';
import type { AbstractServiceOptions, Alterations, MutationOptions, PrimaryKey } from '../types/index.js';
	override async updateOne(key: PrimaryKey, data: Record<string, any>, opts?: MutationOptions): Promise<PrimaryKey> {
	override async updateBatch(data: Record<string, any>[], opts?: MutationOptions): Promise<PrimaryKey[]> {
		const primaryKeyField = this.schema.collections[this.collection]!.primary;
	override async updateMany(
		keys: PrimaryKey[],
		data: Record<string, any>,
		opts?: MutationOptions,
	): Promise<PrimaryKey[]> {
Fixed code: ---
'@directus/api': patch
---

Added validation for IP Access in role settings
import { MockClient, Tracker, createTracker, type RawQuery } from 'knex-mock-client';
	type MockedFunction,

			it('should throw due to invalid ip_access', async () => {
				await expect(service.createOne({ ip_access: ['invalid_ip'] })).rejects.toThrow(
					'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
				);
			});

			it('should throw due to invalid ip_access', async () => {
				await expect(service.createMany([{ ip_access: ['invalid_ip'] }])).rejects.toThrow(
					'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
				);
			});

			it('should throw due to invalid ip_access', async () => {
				await expect(service.updateOne(1, { ip_access: ['invalid_ip'] })).rejects.toThrow(
					'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
				);
			});

			it('should throw due to invalid ip_access', async () => {
				await expect(service.updateMany([1], { ip_access: ['invalid_ip'] })).rejects.toThrow(
					'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
				);
			});

			it('should throw due to invalid ip_access', async () => {
				await expect(service.updateBatch([{ id: 1, ip_access: ['invalid_ip'] }])).rejects.toThrow(
					'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
				);
			});

			it('should throw due to invalid ip_access', async () => {
				await expect(service.updateByQuery({}, { ip_access: ['invalid_ip'] })).rejects.toThrow(
					'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
				);
			});
import { ForbiddenError, InvalidPayloadError, UnprocessableContentError } from '@directus/errors';
import { getMatch } from 'ip-matching';
import type { AbstractServiceOptions, Alterations, Item, MutationOptions, PrimaryKey } from '../types/index.js';
	private isIpAccessValid(value?: any[] | null): boolean {
		if (value === undefined) return false;
		if (value === null) return true;
		if (Array.isArray(value) && value.length === 0) return true;

		for (const ip of value) {
			if (typeof ip !== 'string' || ip.includes('*')) return false;

			try {
				const match = getMatch(ip);
				if (match.type == 'IPMask') return false;
			} catch {
				return false;
			}
		}

		return true;
	}

	private assertValidIpAccess(partialItem: Partial<Item>): void {
		if ('ip_access' in partialItem && !this.isIpAccessValid(partialItem['ip_access'])) {
			throw new InvalidPayloadError({
				reason: 'IP Access contains an incorrect value. Valid values are: IP addresses, IP ranges and CIDR blocks',
			});
		}
	}

	override async createOne(data: Partial<Item>, opts?: MutationOptions): Promise<PrimaryKey> {
		this.assertValidIpAccess(data);

		return super.createOne(data, opts);
	}

	override async createMany(data: Partial<Item>[], opts?: MutationOptions): Promise<PrimaryKey[]> {
		for (const partialItem of data) {
			this.assertValidIpAccess(partialItem);
		}

		return super.createMany(data, opts);
	}

	override async updateOne(key: PrimaryKey, data: Partial<Item>, opts?: MutationOptions): Promise<PrimaryKey> {
		this.assertValidIpAccess(data);

	override async updateBatch(data: Partial<Item>[], opts?: MutationOptions): Promise<PrimaryKey[]> {
		for (const partialItem of data) {
			this.assertValidIpAccess(partialItem);
		}
		const primaryKeyField = this.schema.collections[this.collection]!.primary;
	override async updateMany(keys: PrimaryKey[], data: Partial<Item>, opts?: MutationOptions): Promise<PrimaryKey[]> {
		this.assertValidIpAccess(data);

	override async updateByQuery(
		query: Query,
		data: Partial<Item>,
		opts?: MutationOptions | undefined,
	): Promise<PrimaryKey[]> {
		this.assertValidIpAccess(data);

		return super.updateByQuery(query, data, opts);
	}