Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lovely-bats-buy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@rocket.chat/meteor': patch
---

Security Hotfix (https://docs.rocket.chat/docs/security-fixes-and-updates)
10 changes: 7 additions & 3 deletions apps/meteor/app/threads/server/methods/getThreadMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ Meteor.methods<ServerMethods>({
});
}

if (typeof tmid !== 'string') {
throw new Meteor.Error('error-invalid-message', 'Invalid message', { method: 'getThreadMessages' });
}

const thread = await Messages.findOneById(tmid);
if (!thread) {
return [];
Expand All @@ -48,15 +52,15 @@ Meteor.methods<ServerMethods>({
}

await callbacks.run('beforeReadMessages', thread.rid, user._id);
await readThread({ userId: user._id, rid: thread.rid, tmid });
await readThread({ userId: user._id, rid: thread.rid, tmid: thread._id });

const result = await Messages.findVisibleThreadByThreadId(tmid, {
const result = await Messages.findVisibleThreadByThreadId(thread._id, {
...(skip && { skip }),
...(limit && { limit }),
sort: { ts: -1 },
}).toArray();

callbacks.runAsync('afterReadMessages', room, { uid: user._id, tmid });
callbacks.runAsync('afterReadMessages', room, { uid: user._id, tmid: thread._id });

return [thread, ...result];
},
Expand Down
6 changes: 5 additions & 1 deletion apps/meteor/app/threads/server/methods/getThreadsList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,17 @@ Meteor.methods<ServerMethods>({
throw new Meteor.Error('error-not-allowed', 'Threads Disabled', { method: 'getThreadsList' });
}

if (typeof rid !== 'string') {
throw new Meteor.Error('error-invalid-room', 'Invalid room', { method: 'getThreadsList' });
}

const user = await Meteor.userAsync();
const room = await Rooms.findOneById(rid);

if (!user || !room || !(await canAccessRoomAsync(room, user))) {
throw new Meteor.Error('error-not-allowed', 'Not Allowed', { method: 'getThreadsList' });
}

return Messages.findThreadsByRoomId(rid, skip, limit).toArray();
return Messages.findThreadsByRoomId(room._id, skip, limit).toArray();
},
});
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,11 @@ export class NotificationsModule {
...args: [{ action: string; params: { callId: string; uid: string; rid: string } }] | [IUserDataEvent]
) {
const [roomId, e] = eventName.split('/') as [string, 'video-conference' | 'userData'];
if (this.userId && (await Subscriptions.countByRoomIdAndUserId(roomId, this.userId)) > 0) {
if (
this.userId &&
['video-conference', 'userData'].includes(e) &&
(await Subscriptions.countByRoomIdAndUserId(roomId, this.userId)) > 0
) {
const subscriptions: ISubscription[] = await Subscriptions.findByRoomIdAndNotUserId(roomId, this.userId, {
projection: { 'u._id': 1, '_id': 0 },
}).toArray();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,61 +19,135 @@ class TestStreamer extends Streamer<any> {
}
const validateActionStub = sinon.stub();
const processSerializedSignalStub = sinon.stub();
const countByRoomIdAndUserIdStub = sinon.stub();
const findSubscriptionsExcludingUserStub = sinon.stub();

const { NotificationsModule } = proxyquire.noCallThru().load('../../../../../server/modules/notifications/notifications.module', {
'@rocket.chat/core-services': {
VideoConf: { validateAction: validateActionStub },
MediaCall: { processSerializedSignal: processSerializedSignalStub },
},
'@rocket.chat/models': {
Subscriptions: {
countByRoomIdAndUserId: countByRoomIdAndUserIdStub,
findByRoomIdAndNotUserId: findSubscriptionsExcludingUserStub,
},
Rooms: {},
Users: {},
},
});

describe('NotificationsModule notify-user allowWrite', () => {
describe('NotificationsModule', () => {
let notifications: any;

// `isWriteAllowed` is public on the concrete Streamer class but not on the IStreamer
// interface that `streamUser` is typed as, so cast to reach it.
const writeAllowed = (eventName: string, ...args: unknown[]) =>
(notifications.streamUser as unknown as Streamer<'notify-user'>).isWriteAllowed({ userId: 'userId' } as any, eventName, args);

beforeEach(() => {
validateActionStub.reset();
validateActionStub.resolves(true);
processSerializedSignalStub.reset();
processSerializedSignalStub.resolves(undefined);

notifications = new NotificationsModule(TestStreamer as any);
notifications.configure();
});

afterEach(() => {
Object.keys(StreamerCentral.instances).forEach((name) => delete StreamerCentral.instances[name]);
});
describe('notify-user allowWrite', () => {
// `isWriteAllowed` is public on the concrete Streamer class but not on the IStreamer
// interface that `streamUser` is typed as, so cast to reach it.
const writeAllowed = (eventName: string, ...args: unknown[]) =>
(notifications.streamUser as unknown as Streamer<'notify-user'>).isWriteAllowed({ userId: 'userId' } as any, eventName, args);

beforeEach(() => {
validateActionStub.reset();
validateActionStub.resolves(true);
processSerializedSignalStub.reset();
processSerializedSignalStub.resolves(undefined);
countByRoomIdAndUserIdStub.reset();
findSubscriptionsExcludingUserStub.reset();
});

['force_logout', 'notification', 'message', 'uiInteraction', 'subscriptions-changed', 'webdav', 'banners'].forEach((event) => {
it(`should deny a logged-in client writing "${event}" to another user's stream`, async () => {
expect(await writeAllowed(`victim/${event}`, { foo: 'bar' })).to.equal(false);
afterEach(() => {
Object.keys(StreamerCentral.instances).forEach((name) => delete StreamerCentral.instances[name]);
});
});

it("should deny writes even to the client's own stream", async () => {
expect(await writeAllowed(`userId/force_logout`, undefined)).to.equal(false);
});
['force_logout', 'notification', 'message', 'uiInteraction', 'subscriptions-changed', 'webdav', 'banners'].forEach((event) => {
it(`should deny a logged-in client writing "${event}" to another user's stream`, async () => {
expect(await writeAllowed(`victim/${event}`, { foo: 'bar' })).to.equal(false);
});
});

it("should deny writes even to the client's own stream", async () => {
expect(await writeAllowed(`userId/force_logout`, undefined)).to.equal(false);
});

it('should accept "video-conference" and delegate authorization to VideoConf.validateAction', async () => {
const result = await writeAllowed(`userId/video-conference`, {
action: 'call-start',
params: { callId: '123', uid: '456', rid: '789' },
});

it('should accept "video-conference" and delegate authorization to VideoConf.validateAction', async () => {
const result = await writeAllowed(`userId/video-conference`, {
action: 'call-start',
params: { callId: '123', uid: '456', rid: '789' },
expect(result).to.be.true;
expect(validateActionStub.calledOnceWith('call-start', 'userId', { callId: '123', uid: '456', rid: '789' })).to.be.true;
});

expect(result).to.be.true;
expect(validateActionStub.calledOnceWith('call-start', 'userId', { callId: '123', uid: '456', rid: '789' })).to.be.true;
it('should process "media-calls" signals server-side and never broadcast them', async () => {
const signal = '{"type":"offer"}';
const result = await writeAllowed(`userId/media-calls`, signal);

expect(result).to.equal(false);
expect(processSerializedSignalStub.calledOnceWith('userId', signal)).to.be.true;
});
});

it('should process "media-calls" signals server-side and never broadcast them', async () => {
const signal = '{"type":"offer"}';
const result = await writeAllowed(`userId/media-calls`, signal);
describe('notify-room-users allowWrite', () => {
const writeAllowed = (eventName: string, ...args: unknown[]) =>
(notifications.streamRoomUsers as unknown as Streamer<'notify-room-users'>).isWriteAllowed(
{ userId: 'attacker' } as any,
eventName,
args,
);

beforeEach(() => {
countByRoomIdAndUserIdStub.reset();
countByRoomIdAndUserIdStub.resolves(1); // attacker is subscribed to the room
findSubscriptionsExcludingUserStub.reset();
findSubscriptionsExcludingUserStub.returns({ toArray: async () => [{ u: { _id: 'victim' } }] });
});

expect(result).to.equal(false);
expect(processSerializedSignalStub.calledOnceWith('userId', signal)).to.be.true;
afterEach(() => {
Object.keys(StreamerCentral.instances).forEach((name) => delete StreamerCentral.instances[name]);
});

['force_logout', 'notification', 'message', 'uiInteraction', 'subscriptions-changed', 'webdav', 'banners'].forEach((event) => {
it(`should deny and not relay an arbitrary "${event}" event to other room members`, async () => {
const emitSpy = sinon.spy(notifications.streamUser, 'emit');

const result = await writeAllowed(`room1/${event}`, { foo: 'bar' });

expect(result).to.equal(false);
expect(findSubscriptionsExcludingUserStub.called).to.equal(false);
expect(emitSpy.called).to.equal(false);
});
});

it('should not relay anything for a user not subscribed to the room', async () => {
countByRoomIdAndUserIdStub.resolves(0);
const emitSpy = sinon.spy(notifications.streamUser, 'emit');

const result = await writeAllowed('room1/video-conference', {
action: 'call-start',
params: { callId: '123', uid: 'victim', rid: 'room1' },
});

expect(result).to.equal(false);
expect(findSubscriptionsExcludingUserStub.called).to.equal(false);
expect(emitSpy.called).to.equal(false);
});

['video-conference', 'userData'].forEach((event) => {
it(`should relay "${event}" event to other room members`, async () => {
const emitSpy = sinon.spy(notifications.streamUser, 'emit');

const result = await writeAllowed(`room1/${event}`, { foo: 'bar' });

expect(result).to.equal(false);
expect(findSubscriptionsExcludingUserStub.called).to.equal(true);
expect(emitSpy.called).to.equal(true);
});
});
});
});
Loading