mirror of
https://github.com/n8n-io/n8n.git
synced 2025-01-11 21:07:28 -08:00
fix(core): Prevent XSS in user update endpoints (#10338)
This commit is contained in:
parent
4f392b5e3e
commit
78984986a6
|
@ -1,10 +1,15 @@
|
||||||
import { validate } from 'class-validator';
|
import { ValidationError, validate } from 'class-validator';
|
||||||
import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
|
import type { WorkflowEntity } from '@db/entities/WorkflowEntity';
|
||||||
import type { CredentialsEntity } from '@db/entities/CredentialsEntity';
|
import type { CredentialsEntity } from '@db/entities/CredentialsEntity';
|
||||||
import type { TagEntity } from '@db/entities/TagEntity';
|
import type { TagEntity } from '@db/entities/TagEntity';
|
||||||
import type { User } from '@db/entities/User';
|
import type { User } from '@db/entities/User';
|
||||||
import type { UserRoleChangePayload, UserUpdatePayload } from '@/requests';
|
import type {
|
||||||
|
UserRoleChangePayload,
|
||||||
|
UserSettingsUpdatePayload,
|
||||||
|
UserUpdatePayload,
|
||||||
|
} from '@/requests';
|
||||||
import { BadRequestError } from './errors/response-errors/bad-request.error';
|
import { BadRequestError } from './errors/response-errors/bad-request.error';
|
||||||
|
import { NoXss } from './databases/utils/customValidators';
|
||||||
|
|
||||||
export async function validateEntity(
|
export async function validateEntity(
|
||||||
entity:
|
entity:
|
||||||
|
@ -13,7 +18,8 @@ export async function validateEntity(
|
||||||
| TagEntity
|
| TagEntity
|
||||||
| User
|
| User
|
||||||
| UserUpdatePayload
|
| UserUpdatePayload
|
||||||
| UserRoleChangePayload,
|
| UserRoleChangePayload
|
||||||
|
| UserSettingsUpdatePayload,
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
const errors = await validate(entity);
|
const errors = await validate(entity);
|
||||||
|
|
||||||
|
@ -31,3 +37,37 @@ export async function validateEntity(
|
||||||
}
|
}
|
||||||
|
|
||||||
export const DEFAULT_EXECUTIONS_GET_ALL_LIMIT = 20;
|
export const DEFAULT_EXECUTIONS_GET_ALL_LIMIT = 20;
|
||||||
|
|
||||||
|
class StringWithNoXss {
|
||||||
|
@NoXss()
|
||||||
|
value: string;
|
||||||
|
|
||||||
|
constructor(value: string) {
|
||||||
|
this.value = value;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Temporary solution until we implement payload validation middleware
|
||||||
|
export async function validateRecordNoXss(record: Record<string, string>) {
|
||||||
|
const errors: ValidationError[] = [];
|
||||||
|
|
||||||
|
for (const [key, value] of Object.entries(record)) {
|
||||||
|
const stringWithNoXss = new StringWithNoXss(value);
|
||||||
|
const validationErrors = await validate(stringWithNoXss);
|
||||||
|
|
||||||
|
if (validationErrors.length > 0) {
|
||||||
|
const error = new ValidationError();
|
||||||
|
error.property = key;
|
||||||
|
error.constraints = validationErrors[0].constraints;
|
||||||
|
errors.push(error);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (errors.length > 0) {
|
||||||
|
const errorMessages = errors
|
||||||
|
.map((error) => `${error.property}: ${Object.values(error.constraints ?? {}).join(', ')}`)
|
||||||
|
.join(' | ');
|
||||||
|
|
||||||
|
throw new BadRequestError(errorMessages);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
@ -225,6 +225,26 @@ describe('MeController', () => {
|
||||||
new BadRequestError('Personalization answers are mandatory'),
|
new BadRequestError('Personalization answers are mandatory'),
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should throw BadRequestError on XSS attempt', async () => {
|
||||||
|
const req = mock<MeRequest.SurveyAnswers>({
|
||||||
|
body: { 'test-answer': '<script>alert("XSS")</script>' },
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(controller.storeSurveyAnswers(req)).rejects.toThrowError(BadRequestError);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('updateCurrentUserSettings', () => {
|
||||||
|
it('should throw BadRequestError on XSS attempt', async () => {
|
||||||
|
const req = mock<AuthenticatedRequest>({
|
||||||
|
body: {
|
||||||
|
userActivated: '<script>alert("XSS")</script>',
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(controller.updateCurrentUserSettings(req)).rejects.toThrowError(BadRequestError);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('API Key methods', () => {
|
describe('API Key methods', () => {
|
||||||
|
|
|
@ -6,7 +6,7 @@ import { randomBytes } from 'crypto';
|
||||||
import { AuthService } from '@/auth/auth.service';
|
import { AuthService } from '@/auth/auth.service';
|
||||||
import { Delete, Get, Patch, Post, RestController } from '@/decorators';
|
import { Delete, Get, Patch, Post, RestController } from '@/decorators';
|
||||||
import { PasswordUtility } from '@/services/password.utility';
|
import { PasswordUtility } from '@/services/password.utility';
|
||||||
import { validateEntity } from '@/GenericHelpers';
|
import { validateEntity, validateRecordNoXss } from '@/GenericHelpers';
|
||||||
import type { User } from '@db/entities/User';
|
import type { User } from '@db/entities/User';
|
||||||
import {
|
import {
|
||||||
AuthenticatedRequest,
|
AuthenticatedRequest,
|
||||||
|
@ -176,6 +176,8 @@ export class MeController {
|
||||||
throw new BadRequestError('Personalization answers are mandatory');
|
throw new BadRequestError('Personalization answers are mandatory');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
await validateRecordNoXss(personalizationAnswers);
|
||||||
|
|
||||||
await this.userRepository.save(
|
await this.userRepository.save(
|
||||||
{
|
{
|
||||||
id: req.user.id,
|
id: req.user.id,
|
||||||
|
@ -237,6 +239,9 @@ export class MeController {
|
||||||
const payload = plainToInstance(UserSettingsUpdatePayload, req.body, {
|
const payload = plainToInstance(UserSettingsUpdatePayload, req.body, {
|
||||||
excludeExtraneousValues: true,
|
excludeExtraneousValues: true,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
await validateEntity(payload);
|
||||||
|
|
||||||
const { id } = req.user;
|
const { id } = req.user;
|
||||||
|
|
||||||
await this.userService.updateSettings(id, payload);
|
await this.userService.updateSettings(id, payload);
|
||||||
|
|
Loading…
Reference in a new issue