From 3f707fbeb140cb36dc7226d34688135179d2981f Mon Sep 17 00:00:00 2001 From: Julio A <52619625+julio-cfa@users.noreply.github.com> Date: Fri, 20 Sep 2024 23:54:44 +0200 Subject: [PATCH 1/2] fix: security hotfix --- apps/meteor/app/api/server/v1/rooms.ts | 7 ++++- .../content/urlPreviews/OEmbedHtmlPreview.tsx | 11 +++++++- apps/meteor/tests/end-to-end/api/rooms.ts | 28 +++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/api/server/v1/rooms.ts b/apps/meteor/app/api/server/v1/rooms.ts index 3dc62e462ddf..b9b5709d8fdb 100644 --- a/apps/meteor/app/api/server/v1/rooms.ts +++ b/apps/meteor/app/api/server/v1/rooms.ts @@ -365,7 +365,12 @@ API.v1.addRoute( { authRequired: true, validateParams: isRoomsCleanHistoryProps }, { async post() { - const { _id } = await findRoomByIdOrName({ params: this.bodyParams }); + const room = await findRoomByIdOrName({ params: this.bodyParams }); + const { _id } = room; + + if (!room || !(await canAccessRoomAsync(room, { _id: this.userId }))) { + return API.v1.failure('User does not have access to the room [error-not-allowed]', 'error-not-allowed'); + } const { latest, diff --git a/apps/meteor/client/components/message/content/urlPreviews/OEmbedHtmlPreview.tsx b/apps/meteor/client/components/message/content/urlPreviews/OEmbedHtmlPreview.tsx index e8dd4e1ddcc2..518f1ebad203 100644 --- a/apps/meteor/client/components/message/content/urlPreviews/OEmbedHtmlPreview.tsx +++ b/apps/meteor/client/components/message/content/urlPreviews/OEmbedHtmlPreview.tsx @@ -1,12 +1,21 @@ import { Box } from '@rocket.chat/fuselage'; +import DOMPurify from 'dompurify'; import type { ReactElement } from 'react'; import React from 'react'; import OEmbedCollapsible from './OEmbedCollapsible'; import type { OEmbedPreviewMetadata } from './OEmbedPreviewMetadata'; +const purifyOptions = { + ADD_TAGS: ['iframe'], + ADD_ATTR: ['frameborder', 'allow', 'allowfullscreen', 'scrolling', 'src', 'style', 'referrerpolicy'], + ALLOW_UNKNOWN_PROTOCOLS: true, +}; + const OEmbedHtmlPreview = ({ html, ...props }: OEmbedPreviewMetadata): ReactElement => ( - {html && } + + {html && } + ); export default OEmbedHtmlPreview; diff --git a/apps/meteor/tests/end-to-end/api/rooms.ts b/apps/meteor/tests/end-to-end/api/rooms.ts index 15f85964ffff..5047af7956d8 100644 --- a/apps/meteor/tests/end-to-end/api/rooms.ts +++ b/apps/meteor/tests/end-to-end/api/rooms.ts @@ -1133,6 +1133,34 @@ describe('[Rooms]', () => { }) .end(done); }); + describe('test user is not part of room', async () => { + beforeEach(async () => { + await updatePermission('clean-channel-history', ['admin', 'user']); + }); + + afterEach(async () => { + await updatePermission('clean-channel-history', ['admin']); + }); + + it('should return an error when the user with right privileges is not part of the room', async () => { + await request + .post(api('rooms.cleanHistory')) + .set(userCredentials) + .send({ + roomId: privateChannel._id, + latest: '9999-12-31T23:59:59.000Z', + oldest: '0001-01-01T00:00:00.000Z', + limit: 2000, + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('errorType', 'error-not-allowed'); + expect(res.body).to.have.property('error', 'User does not have access to the room [error-not-allowed]'); + }); + }); + }); }); describe('[/rooms.info]', () => { let testChannel: IRoom; From c162dca733d607a6122992d02c938fb2aed3dbf8 Mon Sep 17 00:00:00 2001 From: Julio Date: Sat, 21 Sep 2024 00:44:12 +0200 Subject: [PATCH 2/2] docs: add changeset --- .changeset/little-bottles-peel.md | 5 +++ apps/meteor/app/api/server/v1/rooms.ts | 2 +- .../lib/server/methods/cleanRoomHistory.ts | 8 ++++ apps/meteor/tests/end-to-end/api/methods.ts | 43 ++++++++++++++++++- 4 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 .changeset/little-bottles-peel.md diff --git a/.changeset/little-bottles-peel.md b/.changeset/little-bottles-peel.md new file mode 100644 index 000000000000..eacb88108a0f --- /dev/null +++ b/.changeset/little-bottles-peel.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': patch +--- + +Security Hotfix (https://docs.rocket.chat/docs/security-fixes-and-updates) diff --git a/apps/meteor/app/api/server/v1/rooms.ts b/apps/meteor/app/api/server/v1/rooms.ts index b9b5709d8fdb..117ae3851c43 100644 --- a/apps/meteor/app/api/server/v1/rooms.ts +++ b/apps/meteor/app/api/server/v1/rooms.ts @@ -40,7 +40,7 @@ import { findRoomsAvailableForTeams, } from '../lib/rooms'; -async function findRoomByIdOrName({ +export async function findRoomByIdOrName({ params, checkedArchived = true, }: { diff --git a/apps/meteor/app/lib/server/methods/cleanRoomHistory.ts b/apps/meteor/app/lib/server/methods/cleanRoomHistory.ts index d6136eee9131..c804128d27bd 100644 --- a/apps/meteor/app/lib/server/methods/cleanRoomHistory.ts +++ b/apps/meteor/app/lib/server/methods/cleanRoomHistory.ts @@ -2,6 +2,8 @@ import type { ServerMethods } from '@rocket.chat/ddp-client'; import { Match, check } from 'meteor/check'; import { Meteor } from 'meteor/meteor'; +import { findRoomByIdOrName } from '../../../api/server/v1/rooms'; +import { canAccessRoomAsync } from '../../../authorization/server'; import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission'; import { cleanRoomHistory } from '../functions/cleanRoomHistory'; @@ -56,6 +58,12 @@ Meteor.methods({ throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'cleanRoomHistory' }); } + const room = await findRoomByIdOrName({ params: { roomId } }); + + if (!room || !(await canAccessRoomAsync(room, { _id: userId }))) { + throw new Meteor.Error('error-not-allowed', 'Not allowed', { method: 'cleanRoomHistory' }); + } + return cleanRoomHistory({ rid: roomId, latest, diff --git a/apps/meteor/tests/end-to-end/api/methods.ts b/apps/meteor/tests/end-to-end/api/methods.ts index 08945994e438..e3c42389e506 100644 --- a/apps/meteor/tests/end-to-end/api/methods.ts +++ b/apps/meteor/tests/end-to-end/api/methods.ts @@ -616,9 +616,19 @@ describe('Meteor.methods', () => { describe('[@cleanRoomHistory]', () => { let rid: IRoom['_id']; - + let testUser: IUser; + let testUserCredentials: Credentials; let channelName: string; + before('update permissions', async () => { + await updatePermission('clean-channel-history', ['admin', 'user']); + }); + + before('create test user', async () => { + testUser = await createUser(); + testUserCredentials = await login(testUser.username, password); + }); + before('create room', (done) => { channelName = `methods-test-channel-${Date.now()}`; void request @@ -676,7 +686,36 @@ describe('Meteor.methods', () => { .end(done); }); - after(() => deleteRoom({ type: 'p', roomId: rid })); + after(() => + Promise.all([deleteRoom({ type: 'p', roomId: rid }), deleteUser(testUser), updatePermission('clean-channel-history', ['admin'])]), + ); + + it('should throw an error if user is not part of the room', async () => { + await request + .post(methodCall('cleanRoomHistory')) + .set(testUserCredentials) + .send({ + message: JSON.stringify({ + method: 'cleanRoomHistory', + params: [ + { + roomId: rid, + oldest: { $date: new Date().getTime() }, + latest: { $date: new Date().getTime() }, + }, + ], + id: 'id', + msg: 'method', + }), + }) + .expect(200) + .expect((res) => { + expect(res.body).to.have.a.property('success', true); + const data = JSON.parse(res.body.message); + expect(data).to.have.a.property('error').that.is.an('object'); + expect(data.error).to.have.a.property('error', 'error-not-allowed'); + }); + }); it('should not change the _updatedAt value when nothing is changed on the room', async () => { const roomBefore = await request.get(api('groups.info')).set(credentials).query({