[pull] develop from RocketChat:develop

.changeset/eighty-onions-deny.md

@@ -0,0 +1,5 @@
+---
+'@rocket.chat/meteor': patch
+---
+
+Fixes error handling when using invalid regular expressions on message search

apps/meteor/server/lib/parseMessageSearchQuery.ts

@@ -250,11 +250,17 @@ class MessageSearchQueryParser {
 
 		if (/^\/.+\/[imxs]*$/.test(text)) {
 			const r = text.split('/');
+
+			// We remove the 'x' flag that JS does not support but Mongo does
+			new RegExp(r[1], r[2].replace(/x/g, ''));
+
 			this.query.msg = {
 				$regex: r[1],
 				$options: r[2],
 			};
 		} else if (this.forceRegex) {
+			new RegExp(text, 'i');
+
 			this.query.msg = {
 				$regex: text,
 				$options: 'i',

apps/meteor/server/methods/messageSearch.ts

@@ -1,5 +1,6 @@
 import type { ISubscription } from '@rocket.chat/core-typings';
 import type { ServerMethods } from '@rocket.chat/ddp-client';
+import { Logger } from '@rocket.chat/logger';
 import { Messages, Subscriptions, Users } from '@rocket.chat/models';
 import { Match, check } from 'meteor/check';
 import { Meteor } from 'meteor/meteor';
@@ -11,6 +12,8 @@ import { settings } from '../../app/settings/server';
 import { readSecondaryPreferred } from '../database/readSecondaryPreferred';
 import { parseMessageSearchQuery } from '../lib/parseMessageSearchQuery';
 
+const logger = new Logger('MessageSearch');
+
 declare module '@rocket.chat/ddp-client' {
 	// eslint-disable-next-line @typescript-eslint/naming-convention
 	interface ServerMethods {
@@ -45,12 +48,24 @@ export const messageSearch = async function (
 
 	const user = (await Users.findOneById(userId)) || undefined;
 
-	const { query, options } = parseMessageSearchQuery(text, {
-		user,
-		offset,
-		limit,
-		forceRegex: settings.get('Message_AlwaysSearchRegExp'),
-	});
+	let parsedQuery: ReturnType<typeof parseMessageSearchQuery>;
+
+	try {
+		parsedQuery = parseMessageSearchQuery(text, {
+			user,
+			offset,
+			limit,
+			forceRegex: settings.get('Message_AlwaysSearchRegExp'),
+		});
+	} catch (error: unknown) {
+		logger.error({ msg: 'Error while parsing message search query', error });
+		if (error instanceof SyntaxError) {
+			return { message: { docs: [] } };
+		}
+		throw error;
+	}
+
+	const { query, options } = parsedQuery;
 
 	if (Object.keys(query).length === 0) {
 		return {
@@ -75,15 +90,20 @@ export const messageSearch = async function (
 		};
 	}
 
-	return {
-		message: {
-			docs: await Messages.find(query, {
-				// @ts-expect-error col.s.db is not typed
-				readPreference: readSecondaryPreferred(Messages.col.s.db),
-				...options,
-			}).toArray(),
-		},
-	};
+	try {
+		return {
+			message: {
+				docs: await Messages.find(query, {
+					// @ts-expect-error col.s.db is not typed
+					readPreference: readSecondaryPreferred(Messages.col.s.db),
+					...options,
+				}).toArray(),
+			},
+		};
+	} catch (error) {
+		logger.error({ msg: 'Error while finding messages', error });
+		throw new Error('error-while-finding-messages', { cause: error });
+	}
 };
 
 Meteor.methods<ServerMethods>({

apps/meteor/tests/end-to-end/api/abac.ts

@@ -19,7 +19,7 @@ import {
 import { updatePermission, updateSetting } from '../../data/permissions.helper';
 import { createRoom, deleteRoom } from '../../data/rooms.helper';
 import { deleteTeam } from '../../data/teams.helper';
-import { adminEmail, password } from '../../data/user';
+import { adminEmail, adminUsername, password } from '../../data/user';
 import { createUser, deleteUser, login } from '../../data/users.helper';
 import { IS_EE, URL_MONGODB } from '../../e2e/config/constants';
 
@@ -47,7 +47,6 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
 	const updatedKey = `${initialKey}_renamed`;
 	const anotherKey = `${initialKey}_another`;
 	let attributeId: string;
-	let paginationBase: string;
 	let page1AttributeIds: string[] = [];
 
 	before((done) => getCredentials(done));
@@ -328,7 +327,7 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
 		});
 
 		it('GET should paginate attributes (page 1)', async () => {
-			paginationBase = `pg_${Date.now()}`;
+			const paginationBase = `pg_${Date.now()}`;
 			await Promise.all(
 				['a', 'b', 'c'].map((suffix) =>
 					request
@@ -455,39 +454,6 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
 					expect(res.body).to.have.property('key', updatedKey);
 				});
 		});
-
-		it('PUT should update key only (key-updated)', async () => {
-			const testKey = `${initialKey}_update_test`;
-			await request
-				.post(`${v1}/abac/attributes`)
-				.set(credentials)
-				.send({ key: testKey, values: ['val1', 'val2'] })
-				.expect(200);
-
-			const listRes = await request.get(`${v1}/abac/attributes`).query({ key: testKey }).set(credentials).expect(200);
-
-			const attr = listRes.body.attributes.find((a: any) => a.key === testKey);
-			expect(attr).to.exist;
-			const attrId = attr._id;
-
-			const newKey = `${initialKey}_update_test_renamed`;
-			await request
-				.put(`${v1}/abac/attributes/${attrId}`)
-				.set(credentials)
-				.send({ key: newKey })
-				.expect(200)
-				.expect((res) => {
-					expect(res.body).to.have.property('success', true);
-				});
-
-			await request
-				.get(`${v1}/abac/attributes/${attrId}`)
-				.set(credentials)
-				.expect(200)
-				.expect((res) => {
-					expect(res.body).to.have.property('key', newKey);
-				});
-		});
 	});
 
 	describe('Room Attribute Operations', () => {
@@ -1350,13 +1316,11 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
 		});
 
 		describe('Room attribute limits (max 10 attribute keys)', () => {
-			const tempAttrKeys: string[] = [];
 			it('Reset room attributes before limit test and populate with 10 keys', async () => {
 				await request.delete(`${v1}/abac/rooms/${testRoom._id}/attributes`).set(credentials).expect(200);
 
 				const timestamp = Date.now();
 				const keys = Array.from({ length: 10 }, (_, i) => `limitk_${timestamp}_${i}`);
-				tempAttrKeys.push(...keys);
 				await Promise.all(
 					keys.map((k) =>
 						request
@@ -1477,6 +1441,16 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
 					});
 			});
 
+			it('DELETE /abac/rooms/:rid/attributes/:key succeeds while disabled (endpoint has no enabled-check)', async () => {
+				await request
+					.delete(`${v1}/abac/rooms/${testRoom._id}/attributes/never_added_${Date.now()}`)
+					.set(credentials)
+					.expect(200)
+					.expect((res) => {
+						expect(res.body).to.have.property('success', true);
+					});
+			});
+
 			after(async () => {
 				await updateSetting('ABAC_Enabled', true);
 			});
@@ -3497,6 +3471,19 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
 				message: 'ABAC_PDP_Health_IdP_Failed',
 			});
 		});
+
+		it('returns 200 ABAC_PDP_Health_OK when platform, IdP, and authorization all succeed', async () => {
+			await seedDefaultMocks();
+			await seedGetEntitlements({});
+
+			const res = await request.get('/api/v1/abac/pdp/health').set(credentials).expect(200);
+
+			expect(res.body).to.deep.include({
+				success: true,
+				available: true,
+				message: 'ABAC_PDP_Health_OK',
+			});
+		});
 	});
 
 	describe('users.info abacAttributes visibility', () => {
@@ -3526,12 +3513,11 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
 		const v1 = '/api/v1';
 		const NS = 'example.com';
 		const fqn = (key: string, value: string) => `https://${NS}/attr/${key}/value/${value}`;
-		const adminPassword = password;
 		let storeConnection: MongoClient;
 
 		const makeAdmin = async (slug: string) => {
 			const u = await createUser({ roles: ['admin'], username: `vstore-${slug}-${Date.now()}-${Math.random().toString(36).slice(2, 7)}` });
-			const creds = await login(u.username, adminPassword);
+			const creds = await login(u.username, password);
 			return { user: u, creds };
 		};
 
@@ -4173,6 +4159,45 @@ const addAbacAttributesToUserDirectly = async (userId: string, abacAttributes: I
 			});
 		});
 
+		describe('GET /abac/audit query filters', () => {
+			it('scopes events to the actor passed in the query', async () => {
+				const mine = await request
+					.get(`${v1}/abac/audit`)
+					.set(credentials)
+					.query({ count: 100, actor: { username: adminUsername } })
+					.expect(200);
+				expect(mine.body.events).to.be.an('array').that.is.not.empty;
+				(mine.body.events as Array<{ actor?: { username?: string } }>).forEach((e) => {
+					expect(e.actor?.username).to.equal(adminUsername);
+				});
+
+				const other = await request
+					.get(`${v1}/abac/audit`)
+					.set(credentials)
+					.query({ count: 100, actor: { _id: 'no-such-actor-xyz' } })
+					.expect(200);
+				expect(other.body.events).to.be.an('array').that.is.empty;
+				expect(other.body.total).to.equal(0);
+			});
+
+			it('returns no events when start is in the future', async () => {
+				const future = new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString();
+				const res = await request.get(`${v1}/abac/audit`).set(credentials).query({ count: 100, start: future }).expect(200);
+				expect(res.body.events).to.be.an('array').that.is.empty;
+				expect(res.body.total).to.equal(0);
+			});
+
+			it('returns no events when end precedes all events', async () => {
+				const res = await request
+					.get(`${v1}/abac/audit`)
+					.set(credentials)
+					.query({ count: 100, end: new Date(1).toISOString() })
+					.expect(200);
+				expect(res.body.events).to.be.an('array').that.is.empty;
+				expect(res.body.total).to.equal(0);
+			});
+		});
+
 		describe('local-mode regression (ABAC_Attribute_Store=local)', () => {
 			const localKey = `vstore_local_${Date.now()}`;
 			let localRoom: IRoom;

apps/meteor/tests/end-to-end/api/chat.ts

@@ -2539,6 +2539,7 @@ describe('[Chat]', () => {
 			await sendMessage('msg1');
 			await sendMessage('msg1');
 			await sendMessage('msg1');
+			await sendMessage('aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa!');
 		});
 
 		it('should return a list of messages when execute successfully', (done) => {
@@ -2614,6 +2615,70 @@ describe('[Chat]', () => {
 				})
 				.end(done);
 		});
+
+		it('should return an empty array of messages with status 200 if the regexp starts with an invalid quantifier', async () => {
+			await request
+				.get(api('chat.search'))
+				.set(credentials)
+				.query({
+					roomId: testChannel._id,
+					searchText: '/*test/',
+				})
+				.expect('Content-Type', 'application/json')
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+					expect(res.body).to.have.property('messages').and.to.be.deep.equal([]);
+				});
+		});
+
+		it('should return an empty array of messages with status 200 if the regexp has an unclosed parenthesis', async () => {
+			await request
+				.get(api('chat.search'))
+				.set(credentials)
+				.query({
+					roomId: testChannel._id,
+					searchText: '/(test/',
+				})
+				.expect('Content-Type', 'application/json')
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+					expect(res.body).to.have.property('messages').and.to.be.deep.equal([]);
+				});
+		});
+
+		it('should return an empty array of messages with status 200 if the regexp has an unclosed character class', async () => {
+			await request
+				.get(api('chat.search'))
+				.set(credentials)
+				.query({
+					roomId: testChannel._id,
+					searchText: '/[a-z/',
+				})
+				.expect('Content-Type', 'application/json')
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+					expect(res.body).to.have.property('messages').and.to.be.deep.equal([]);
+				});
+		});
+
+		it('should not cause catastrophic backtracking when using a malicious regexp', async () => {
+			await request
+				.get(api('chat.search'))
+				.set(credentials)
+				.query({
+					roomId: testChannel._id,
+					searchText: '/(a+)+$/',
+				})
+				.expect('Content-Type', 'application/json')
+				.expect(200)
+				.expect((res) => {
+					expect(res.body).to.have.property('success', true);
+					expect(res.body).to.have.property('messages').and.to.be.deep.equal([]);
+				});
+		});
 	});
 
 	describe('[/chat.react]', () => {