CVE-2023-28443: Improper Access Control (CWE-284)
value: Directus is a real-time API and App dashboard for managing SQL database content. Prior to version 9.23.3, the `directus_refresh_token` is not redacted properly from the log outputs and can be used to impersonate users without their permission. This issue is patched in version 9.23.3.
CWE description: The product does not restrict or incorrectly restricts access to a resource from an unauthorized actor.
CVSS v3 base score: 5.5
Published: 2023-03-24T00:15Z
Repository: https://github.com/directus/directus
Language: TypeScript
Files changed: api/src/logger.ts, api/src/utils/redact-header-cookies.test.ts, api/src/utils/redact-header-cookies.ts
Commit date: 2023-03-23 18:54:02 +0100
Commit message: Merge pull request from GHSA-8vg2-wf3q-mwv7

* fix(api): redact header cookie

This is a quick PoC for a fix. I am not sure if it's the best answer and have not added tests yet. If we feel good about it, I can add tests and open a PR. Please let me know how you'd like to proceed!

* cleaner

* rework to handle multiple inputs and add unit tests

* Added same redacting logic for teh response set-cookie

---------

Co-authored-by: Brainslug <tim@brainslug.nl>
Co-authored-by: Brainslug <br41nslug@users.noreply.github.com>
Diff: diff --git a/api/src/logger.ts b/api/src/logger.ts
index 624de522b9..cf10aa593c 100644
--- a/api/src/logger.ts
+++ b/api/src/logger.ts
@@ -1,13 +1,14 @@
 import { toArray } from '@directus/shared/utils';
-import type { Request, RequestHandler } from 'express';
 import { merge } from 'lodash';
-import pino, { LoggerOptions } from 'pino';
+import pino, { LoggerOptions, SerializedResponse } from 'pino';
+import type { Request, RequestHandler } from 'express';
 import pinoHTTP, { stdSerializers } from 'pino-http';
 import { URL } from 'url';
 import env from './env';
 import { getConfigFromEnv } from './utils/get-config-from-env';
+import { redactHeaderCookie } from './utils/redact-header-cookies';
 
 const pinoOptions: LoggerOptions = {
 	level: env.LOG_LEVEL || 'info',
 	redact: {
 		paths: ['req.headers.authorization', `req.cookies.${env.REFRESH_TOKEN_COOKIE_NAME}`],
@@ -84,12 +85,27 @@ export const expressLogger = pinoHTTP({
 	...httpLoggerEnvConfig,
 	serializers: {
 		req(request: Request) {
 			const output = stdSerializers.req(request);
 			output.url = redactQuery(output.url);
+			if (output.headers?.cookie) {
+				output.headers.cookie = redactHeaderCookie(output.headers.cookie, [
+					'access_token',
+					`${env.REFRESH_TOKEN_COOKIE_NAME}`,
+				]);
+			}
 			return output;
 		},
+		res(response: SerializedResponse) {
+			if (response.headers?.['set-cookie']) {
+				response.headers['set-cookie'] = redactHeaderCookie(response.headers['set-cookie'], [
+					'access_token',
+					`${env.REFRESH_TOKEN_COOKIE_NAME}`,
+				]);
+			}
+			return response;
+		},
 	},
 }) as RequestHandler;
 
 export default logger;
 
diff --git a/api/src/utils/redact-header-cookies.test.ts b/api/src/utils/redact-header-cookies.test.ts
new file mode 100644
index 0000000000..17a9aad99e
--- /dev/null
+++ b/api/src/utils/redact-header-cookies.test.ts
@@ -0,0 +1,42 @@
+import { describe, expect, test } from 'vitest';
+import env from '../env';
+import { redactHeaderCookie } from './redact-header-cookies';
+
+describe('redactHeaderCookie', () => {
+	describe('Given auth cookies', () => {
+		test('When it finds a refresh_token, it should redact the value', () => {
+			const tokenKey = env.REFRESH_TOKEN_COOKIE_NAME;
+			const cookieHeader = `${tokenKey}=shh;`;
+			const cookieNames = [`${tokenKey}`];
+
+			const redactedCookie = redactHeaderCookie(cookieHeader, cookieNames);
+			expect(redactedCookie).toBe(`${tokenKey}=--redacted--;`);
+		});
+		test('When it finds an access_token, it should redact the value', () => {
+			const tokenKey = 'access_token';
+			const cookieHeader = `${tokenKey}=secret;`;
+			const cookieNames = [`${tokenKey}`];
+
+			const redactedCookie = redactHeaderCookie(cookieHeader, cookieNames);
+			expect(redactedCookie).toBe(`${tokenKey}=--redacted--;`);
+		});
+		test('When it finds both an access_token and refresh_token, it should redact both values', () => {
+			const cookieHeader = `access_token=secret; ${env.REFRESH_TOKEN_COOKIE_NAME}=shhhhhhh; randomCookie=Erdtree;`;
+			const cookieNames = ['access_token', `${env.REFRESH_TOKEN_COOKIE_NAME}`];
+
+			const redactedCookie = redactHeaderCookie(cookieHeader, cookieNames);
+			expect(redactedCookie).toBe(
+				`access_token=--redacted--; ${env.REFRESH_TOKEN_COOKIE_NAME}=--redacted--; randomCookie=Erdtree;`
+			);
+		});
+	});
+	describe('Given negligible cookies', () => {
+		test('It should return the orignal value', () => {
+			const originalCookie = `Crown=Swords; Hail=Sithis;`;
+			const cookieNames = [env.REFRESH_TOKEN_COOKIE_NAME, 'access_token'];
+
+			const redactedCookie = redactHeaderCookie(originalCookie, cookieNames);
+			expect(redactedCookie).toBe(originalCookie);
+		});
+	});
+});
diff --git a/api/src/utils/redact-header-cookies.ts b/api/src/utils/redact-header-cookies.ts
new file mode 100644
index 0000000000..f5fd73f789
--- /dev/null
+++ b/api/src/utils/redact-header-cookies.ts
@@ -0,0 +1,7 @@
+export function redactHeaderCookie(cookieHeader: string, cookieNames: string[]) {
+	for (const cookieName of cookieNames) {
+		const re = new RegExp(`(${cookieName}=)([^;]+)`);
+		cookieHeader = cookieHeader.replace(re, `$1--redacted--`);
+	}
+	return cookieHeader;
+}

Vulnerable code: import type { Request, RequestHandler } from 'express';
import pino, { LoggerOptions } from 'pino';
Fixed code: import pino, { LoggerOptions, SerializedResponse } from 'pino';
import type { Request, RequestHandler } from 'express';
import { redactHeaderCookie } from './utils/redact-header-cookies';
			if (output.headers?.cookie) {
				output.headers.cookie = redactHeaderCookie(output.headers.cookie, [
					'access_token',
					`${env.REFRESH_TOKEN_COOKIE_NAME}`,
				]);
			}
		res(response: SerializedResponse) {
			if (response.headers?.['set-cookie']) {
				response.headers['set-cookie'] = redactHeaderCookie(response.headers['set-cookie'], [
					'access_token',
					`${env.REFRESH_TOKEN_COOKIE_NAME}`,
				]);
			}
			return response;
		},
import { describe, expect, test } from 'vitest';
import env from '../env';
import { redactHeaderCookie } from './redact-header-cookies';

describe('redactHeaderCookie', () => {
	describe('Given auth cookies', () => {
		test('When it finds a refresh_token, it should redact the value', () => {
			const tokenKey = env.REFRESH_TOKEN_COOKIE_NAME;
			const cookieHeader = `${tokenKey}=shh;`;
			const cookieNames = [`${tokenKey}`];

			const redactedCookie = redactHeaderCookie(cookieHeader, cookieNames);
			expect(redactedCookie).toBe(`${tokenKey}=--redacted--;`);
		});
		test('When it finds an access_token, it should redact the value', () => {
			const tokenKey = 'access_token';
			const cookieHeader = `${tokenKey}=secret;`;
			const cookieNames = [`${tokenKey}`];

			const redactedCookie = redactHeaderCookie(cookieHeader, cookieNames);
			expect(redactedCookie).toBe(`${tokenKey}=--redacted--;`);
		});
		test('When it finds both an access_token and refresh_token, it should redact both values', () => {
			const cookieHeader = `access_token=secret; ${env.REFRESH_TOKEN_COOKIE_NAME}=shhhhhhh; randomCookie=Erdtree;`;
			const cookieNames = ['access_token', `${env.REFRESH_TOKEN_COOKIE_NAME}`];

			const redactedCookie = redactHeaderCookie(cookieHeader, cookieNames);
			expect(redactedCookie).toBe(
				`access_token=--redacted--; ${env.REFRESH_TOKEN_COOKIE_NAME}=--redacted--; randomCookie=Erdtree;`
			);
		});
	});
	describe('Given negligible cookies', () => {
		test('It should return the orignal value', () => {
			const originalCookie = `Crown=Swords; Hail=Sithis;`;
			const cookieNames = [env.REFRESH_TOKEN_COOKIE_NAME, 'access_token'];

			const redactedCookie = redactHeaderCookie(originalCookie, cookieNames);
			expect(redactedCookie).toBe(originalCookie);
		});
	});
});
export function redactHeaderCookie(cookieHeader: string, cookieNames: string[]) {
	for (const cookieName of cookieNames) {
		const re = new RegExp(`(${cookieName}=)([^;]+)`);
		cookieHeader = cookieHeader.replace(re, `$1--redacted--`);
	}
	return cookieHeader;
}