fix(core): Fix XSS validation and separate URL validation (#10424)

packages/cli/package.json

@@ -155,6 +155,7 @@
     "reflect-metadata": "0.2.2",
     "replacestream": "4.0.3",
     "samlify": "2.8.9",
+    "sanitize-html": "2.12.1",
     "semver": "7.5.4",
     "shelljs": "0.8.5",
     "simple-git": "3.17.0",

packages/cli/src/GenericHelpers.ts

@@ -9,7 +9,7 @@ import type {
 	UserUpdatePayload,
 } from '@/requests';
 import { BadRequestError } from './errors/response-errors/bad-request.error';
-import { NoXss } from './databases/utils/customValidators';
+import { NoXss } from '@/validators/no-xss.validator';
 
 export async function validateEntity(
 	entity:

packages/cli/src/databases/entities/User.ts

@@ -13,7 +13,7 @@ import { IsEmail, IsString, Length } from 'class-validator';
 import type { IUser, IUserSettings } from 'n8n-workflow';
 import type { SharedWorkflow } from './SharedWorkflow';
 import type { SharedCredentials } from './SharedCredentials';
-import { NoXss } from '../utils/customValidators';
+import { NoXss } from '@/validators/no-xss.validator';
 import { objectRetriever, lowerCaser } from '../utils/transformers';
 import { WithTimestamps, jsonColumnType } from './AbstractEntity';
 import type { IPersonalizationSurveyAnswers } from '@/Interfaces';
@@ -25,6 +25,7 @@ import {
 } from '@/permissions/global-roles';
 import { hasScope, type ScopeOptions, type Scope } from '@n8n/permissions';
 import type { ProjectRelation } from './ProjectRelation';
+import { NoUrl } from '@/validators/no-url.validator';
 
 export type GlobalRole = 'global:owner' | 'global:admin' | 'global:member';
 export type AssignableRole = Exclude<GlobalRole, 'global:owner'>;
@@ -51,12 +52,14 @@ export class User extends WithTimestamps implements IUser {
 
 	@Column({ length: 32, nullable: true })
 	@NoXss()
+	@NoUrl()
 	@IsString({ message: 'First name must be of type string.' })
 	@Length(1, 32, { message: 'First name must be $constraint1 to $constraint2 characters long.' })
 	firstName: string;
 
 	@Column({ length: 32, nullable: true })
 	@NoXss()
+	@NoUrl()
 	@IsString({ message: 'Last name must be of type string.' })
 	@Length(1, 32, { message: 'Last name must be $constraint1 to $constraint2 characters long.' })
 	lastName: string;

packages/cli/src/databases/utils/__tests__/customValidators.test.ts

@@ -1,43 +0,0 @@
-import { NoXss } from '@db/utils/customValidators';
-import { validate } from 'class-validator';
-
-describe('customValidators', () => {
-	describe('NoXss', () => {
-		class Person {
-			@NoXss()
-			name: string;
-		}
-		const person = new Person();
-
-		const invalidNames = ['http://google.com', '<script src/>', 'www.domain.tld'];
-
-		const validNames = [
-			'Johann Strauß',
-			'Вагиф Сәмәдоғлу',
-			'René Magritte',
-			'সুকুমার রায়',
-			'མགོན་པོ་རྡོ་རྗེ།',
-			'عبدالحليم حافظ',
-		];
-
-		describe('Block XSS', () => {
-			for (const name of invalidNames) {
-				test(name, async () => {
-					person.name = name;
-					const validationErrors = await validate(person);
-					expect(validationErrors[0].property).toEqual('name');
-					expect(validationErrors[0].constraints).toEqual({ NoXss: 'Malicious name' });
-				});
-			}
-		});
-
-		describe('Allow Valid names', () => {
-			for (const name of validNames) {
-				test(name, async () => {
-					person.name = name;
-					expect(await validate(person)).toBeEmptyArray();
-				});
-			}
-		});
-	});
-});

packages/cli/src/databases/utils/customValidators.ts

@@ -1,18 +0,0 @@
-import { registerDecorator } from 'class-validator';
-
-export function NoXss() {
-	return (object: object, propertyName: string): void => {
-		registerDecorator({
-			name: 'NoXss',
-			target: object.constructor,
-			propertyName,
-			constraints: [propertyName],
-			options: { message: `Malicious ${propertyName}` },
-			validator: {
-				validate(value: string) {
-					return !/(^http|^www)|<(\s*)?(script|a)|(\.[\p{L}\d-]+)/u.test(value);
-				},
-			},
-		});
-	};
-}

packages/cli/src/requests.ts

@@ -13,7 +13,7 @@ import type {
 
 import { Expose } from 'class-transformer';
 import { IsBoolean, IsEmail, IsIn, IsOptional, IsString, Length } from 'class-validator';
-import { NoXss } from '@db/utils/customValidators';
+import { NoXss } from '@/validators/no-xss.validator';
 import type { PublicUser, SecretsProvider, SecretsProviderState } from '@/Interfaces';
 import { AssignableRole } from '@db/entities/User';
 import type { GlobalRole, User } from '@db/entities/User';
@@ -26,6 +26,7 @@ import type { ProjectRole } from './databases/entities/ProjectRelation';
 import type { Scope } from '@n8n/permissions';
 import type { ScopesField } from './services/role.service';
 import type { AiAssistantSDK } from '@n8n_io/ai-assistant-sdk';
+import { NoUrl } from '@/validators/no-url.validator';
 
 export class UserUpdatePayload implements Pick<User, 'email' | 'firstName' | 'lastName'> {
 	@Expose()
@@ -34,12 +35,14 @@ export class UserUpdatePayload implements Pick<User, 'email' | 'firstName' | 'la
 
 	@Expose()
 	@NoXss()
+	@NoUrl()
 	@IsString({ message: 'First name must be of type string.' })
 	@Length(1, 32, { message: 'First name must be $constraint1 to $constraint2 characters long.' })
 	firstName: string;
 
 	@Expose()
 	@NoXss()
+	@NoUrl()
 	@IsString({ message: 'Last name must be of type string.' })
 	@Length(1, 32, { message: 'Last name must be $constraint1 to $constraint2 characters long.' })
 	lastName: string;

packages/cli/src/validators/__tests__/no-url.validator.test.ts

@@ -0,0 +1,26 @@
+import { NoUrl } from '../no-url.validator';
+import { validate } from 'class-validator';
+
+describe('NoUrl', () => {
+	class Entity {
+		@NoUrl()
+		name = '';
+	}
+
+	const entity = new Entity();
+
+	describe('URLs', () => {
+		const URLS = ['http://google.com', 'www.domain.tld'];
+
+		for (const str of URLS) {
+			test(`should block ${str}`, async () => {
+				entity.name = str;
+				const errors = await validate(entity);
+				expect(errors).toHaveLength(1);
+				const [error] = errors;
+				expect(error.property).toEqual('name');
+				expect(error.constraints).toEqual({ NoUrl: 'Potentially malicious string' });
+			});
+		}
+	});
+});

packages/cli/src/validators/__tests__/no-xss.validator.test.ts

@@ -0,0 +1,72 @@
+import { NoXss } from '../no-xss.validator';
+import { validate } from 'class-validator';
+
+describe('NoXss', () => {
+	class Entity {
+		@NoXss()
+		name = '';
+
+		@NoXss()
+		timestamp = '';
+
+		@NoXss()
+		version = '';
+	}
+
+	const entity = new Entity();
+
+	describe('Scripts', () => {
+		const XSS_STRINGS = ['<script src/>', "<script>alert('xss')</script>"];
+
+		for (const str of XSS_STRINGS) {
+			test(`should block ${str}`, async () => {
+				entity.name = str;
+				const errors = await validate(entity);
+				expect(errors).toHaveLength(1);
+				const [error] = errors;
+				expect(error.property).toEqual('name');
+				expect(error.constraints).toEqual({ NoXss: 'Potentially malicious string' });
+			});
+		}
+	});
+
+	describe('Names', () => {
+		const VALID_NAMES = [
+			'Johann Strauß',
+			'Вагиф Сәмәдоғлу',
+			'René Magritte',
+			'সুকুমার রায়',
+			'མགོན་པོ་རྡོ་རྗེ།',
+			'عبدالحليم حافظ',
+		];
+
+		for (const name of VALID_NAMES) {
+			test(`should allow ${name}`, async () => {
+				entity.name = name;
+				expect(await validate(entity)).toBeEmptyArray();
+			});
+		}
+	});
+
+	describe('ISO-8601 timestamps', () => {
+		const VALID_TIMESTAMPS = ['2022-01-01T00:00:00.000Z', '2022-01-01T00:00:00.000+02:00'];
+
+		for (const timestamp of VALID_TIMESTAMPS) {
+			test(`should allow ${timestamp}`, async () => {
+				entity.timestamp = timestamp;
+				await expect(validate(entity)).resolves.toBeEmptyArray();
+			});
+		}
+	});
+
+	describe('Semver versions', () => {
+		const VALID_VERSIONS = ['1.0.0', '1.0.0-alpha.1'];
+
+		for (const version of VALID_VERSIONS) {
+			test(`should allow ${version}`, async () => {
+				entity.version = version;
+				await expect(validate(entity)).resolves.toBeEmptyArray();
+			});
+		}
+	});
+});

packages/cli/src/validators/no-url.validator.ts

@@ -0,0 +1,27 @@
+import type { ValidationOptions, ValidatorConstraintInterface } from 'class-validator';
+import { registerDecorator, ValidatorConstraint } from 'class-validator';
+
+const URL_REGEX = /^(https?:\/\/|www\.)/i;
+
+@ValidatorConstraint({ name: 'NoUrl', async: false })
+class NoUrlConstraint implements ValidatorConstraintInterface {
+	validate(value: string) {
+		return !URL_REGEX.test(value);
+	}
+
+	defaultMessage() {
+		return 'Potentially malicious string';
+	}
+}
+
+export function NoUrl(options?: ValidationOptions) {
+	return function (object: object, propertyName: string) {
+		registerDecorator({
+			name: 'NoUrl',
+			target: object.constructor,
+			propertyName,
+			options,
+			validator: NoUrlConstraint,
+		});
+	};
+}

packages/cli/src/validators/no-xss.validator.ts

@@ -0,0 +1,26 @@
+import type { ValidationOptions, ValidatorConstraintInterface } from 'class-validator';
+import { registerDecorator, ValidatorConstraint } from 'class-validator';
+import sanitizeHtml from 'sanitize-html';
+
+@ValidatorConstraint({ name: 'NoXss', async: false })
+class NoXssConstraint implements ValidatorConstraintInterface {
+	validate(value: string) {
+		return value === sanitizeHtml(value, { allowedTags: [], allowedAttributes: {} });
+	}
+
+	defaultMessage() {
+		return 'Potentially malicious string';
+	}
+}
+
+export function NoXss(options?: ValidationOptions) {
+	return function (object: object, propertyName: string) {
+		registerDecorator({
+			name: 'NoXss',
+			target: object.constructor,
+			propertyName,
+			options,
+			validator: NoXssConstraint,
+		});
+	};
+}

pnpm-lock.yaml

@@ -824,6 +824,9 @@ importers:
       samlify:
         specifier: 2.8.9
         version: 2.8.9
+      sanitize-html:
+        specifier: 2.12.1
+        version: 2.12.1
       semver:
         specifier: ^7.5.4
         version: 7.6.0
@@ -22445,7 +22448,7 @@ snapshots:
       domelementtype: 2.3.0
       domhandler: 5.0.3
       domutils: 3.0.1
-      entities: 4.4.0
+      entities: 4.5.0
 
   http-cache-semantics@4.1.1:
     optional: true