Skip to content

Commit ea163f5

Browse files
fix: imported fixes 08-18-2026 (#41814)
Co-authored-by: jonas_florencio <79267723+jonasflorencio@users.noreply.github.com>
1 parent eb6eccc commit ea163f5

5 files changed

Lines changed: 127 additions & 36 deletions

File tree

.changeset/lovely-bats-buy.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@rocket.chat/meteor': patch
3+
---
4+
5+
Security Hotfix (https://docs.rocket.chat/docs/security-fixes-and-updates)

apps/meteor/server/meteor-methods/messages/getThreadMessages.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ Meteor.methods<ServerMethods>({
3434
});
3535
}
3636

37+
if (typeof tmid !== 'string') {
38+
throw new Meteor.Error('error-invalid-message', 'Invalid message', { method: 'getThreadMessages' });
39+
}
40+
3741
const thread = await Messages.findOneById(tmid);
3842
if (!thread) {
3943
return [];
@@ -51,9 +55,9 @@ Meteor.methods<ServerMethods>({
5155
}
5256

5357
await callbacks.run('beforeReadMessages', thread.rid, user._id);
54-
await readThread({ user: user as IUser, room, tmid });
58+
await readThread({ user: user as IUser, room, tmid: thread._id });
5559

56-
const result = await Messages.findVisibleThreadByThreadId(tmid, {
60+
const result = await Messages.findVisibleThreadByThreadId(thread._id, {
5761
...(skip && { skip }),
5862
...(limit && { limit }),
5963
sort: { ts: -1 },

apps/meteor/server/meteor-methods/messages/getThreadsList.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,17 @@ Meteor.methods<ServerMethods>({
2929
throw new Meteor.Error('error-not-allowed', 'Threads Disabled', { method: 'getThreadsList' });
3030
}
3131

32+
if (typeof rid !== 'string') {
33+
throw new Meteor.Error('error-invalid-room', 'Invalid room', { method: 'getThreadsList' });
34+
}
35+
3236
const user = await Meteor.userAsync();
3337
const room = await Rooms.findOneById(rid);
3438

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

39-
return Messages.findThreadsByRoomId(rid, skip, limit).toArray();
43+
return Messages.findThreadsByRoomId(room._id, skip, limit).toArray();
4044
},
4145
});

apps/meteor/server/modules/notifications/notifications.module.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,11 @@ export class NotificationsModule {
250250
...args: [{ action: string; params: { callId: string; uid: string; rid: string } }] | [IUserDataEvent]
251251
) {
252252
const [roomId, e] = eventName.split('/') as [string, 'video-conference' | 'userData'];
253-
if (this.userId && (await Subscriptions.countByRoomIdAndUserId(roomId, this.userId)) > 0) {
253+
if (
254+
this.userId &&
255+
['video-conference', 'userData'].includes(e) &&
256+
(await Subscriptions.countByRoomIdAndUserId(roomId, this.userId)) > 0
257+
) {
254258
const subscriptions: ISubscription[] = await Subscriptions.findByRoomIdAndNotUserId(roomId, this.userId, {
255259
projection: { 'u._id': 1, '_id': 0 },
256260
}).toArray();

apps/meteor/tests/unit/server/modules/notifications/notifications.module.spec.ts

Lines changed: 106 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -19,61 +19,135 @@ class TestStreamer extends Streamer<any> {
1919
}
2020
const validateActionStub = sinon.stub();
2121
const processSerializedSignalStub = sinon.stub();
22+
const countByRoomIdAndUserIdStub = sinon.stub();
23+
const findSubscriptionsExcludingUserStub = sinon.stub();
2224

2325
const { NotificationsModule } = proxyquire.noCallThru().load('../../../../../server/modules/notifications/notifications.module', {
2426
'@rocket.chat/core-services': {
2527
VideoConf: { validateAction: validateActionStub },
2628
MediaCall: { processSerializedSignal: processSerializedSignalStub },
2729
},
30+
'@rocket.chat/models': {
31+
Subscriptions: {
32+
countByRoomIdAndUserId: countByRoomIdAndUserIdStub,
33+
findByRoomIdAndNotUserId: findSubscriptionsExcludingUserStub,
34+
},
35+
Rooms: {},
36+
Users: {},
37+
},
2838
});
2939

30-
describe('NotificationsModule notify-user allowWrite', () => {
40+
describe('NotificationsModule', () => {
3141
let notifications: any;
3242

33-
// `isWriteAllowed` is public on the concrete Streamer class but not on the IStreamer
34-
// interface that `streamUser` is typed as, so cast to reach it.
35-
const writeAllowed = (eventName: string, ...args: unknown[]) =>
36-
(notifications.streamUser as unknown as Streamer<'notify-user'>).isWriteAllowed({ userId: 'userId' } as any, eventName, args);
37-
3843
beforeEach(() => {
39-
validateActionStub.reset();
40-
validateActionStub.resolves(true);
41-
processSerializedSignalStub.reset();
42-
processSerializedSignalStub.resolves(undefined);
43-
4444
notifications = new NotificationsModule(TestStreamer as any);
4545
notifications.configure();
4646
});
4747

48-
afterEach(() => {
49-
Object.keys(StreamerCentral.instances).forEach((name) => delete StreamerCentral.instances[name]);
50-
});
48+
describe('notify-user allowWrite', () => {
49+
// `isWriteAllowed` is public on the concrete Streamer class but not on the IStreamer
50+
// interface that `streamUser` is typed as, so cast to reach it.
51+
const writeAllowed = (eventName: string, ...args: unknown[]) =>
52+
(notifications.streamUser as unknown as Streamer<'notify-user'>).isWriteAllowed({ userId: 'userId' } as any, eventName, args);
53+
54+
beforeEach(() => {
55+
validateActionStub.reset();
56+
validateActionStub.resolves(true);
57+
processSerializedSignalStub.reset();
58+
processSerializedSignalStub.resolves(undefined);
59+
countByRoomIdAndUserIdStub.reset();
60+
findSubscriptionsExcludingUserStub.reset();
61+
});
5162

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

58-
it("should deny writes even to the client's own stream", async () => {
59-
expect(await writeAllowed(`userId/force_logout`, undefined)).to.equal(false);
60-
});
67+
['force_logout', 'notification', 'message', 'uiInteraction', 'subscriptions-changed', 'webdav', 'banners'].forEach((event) => {
68+
it(`should deny a logged-in client writing "${event}" to another user's stream`, async () => {
69+
expect(await writeAllowed(`victim/${event}`, { foo: 'bar' })).to.equal(false);
70+
});
71+
});
72+
73+
it("should deny writes even to the client's own stream", async () => {
74+
expect(await writeAllowed(`userId/force_logout`, undefined)).to.equal(false);
75+
});
76+
77+
it('should accept "video-conference" and delegate authorization to VideoConf.validateAction', async () => {
78+
const result = await writeAllowed(`userId/video-conference`, {
79+
action: 'call-start',
80+
params: { callId: '123', uid: '456', rid: '789' },
81+
});
6182

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

68-
expect(result).to.be.true;
69-
expect(validateActionStub.calledOnceWith('call-start', 'userId', { callId: '123', uid: '456', rid: '789' })).to.be.true;
87+
it('should process "media-calls" signals server-side and never broadcast them', async () => {
88+
const signal = '{"type":"offer"}';
89+
const result = await writeAllowed(`userId/media-calls`, signal);
90+
91+
expect(result).to.equal(false);
92+
expect(processSerializedSignalStub.calledOnceWith('userId', signal)).to.be.true;
93+
});
7094
});
7195

72-
it('should process "media-calls" signals server-side and never broadcast them', async () => {
73-
const signal = '{"type":"offer"}';
74-
const result = await writeAllowed(`userId/media-calls`, signal);
96+
describe('notify-room-users allowWrite', () => {
97+
const writeAllowed = (eventName: string, ...args: unknown[]) =>
98+
(notifications.streamRoomUsers as unknown as Streamer<'notify-room-users'>).isWriteAllowed(
99+
{ userId: 'attacker' } as any,
100+
eventName,
101+
args,
102+
);
103+
104+
beforeEach(() => {
105+
countByRoomIdAndUserIdStub.reset();
106+
countByRoomIdAndUserIdStub.resolves(1); // attacker is subscribed to the room
107+
findSubscriptionsExcludingUserStub.reset();
108+
findSubscriptionsExcludingUserStub.returns({ toArray: async () => [{ u: { _id: 'victim' } }] });
109+
});
75110

76-
expect(result).to.equal(false);
77-
expect(processSerializedSignalStub.calledOnceWith('userId', signal)).to.be.true;
111+
afterEach(() => {
112+
Object.keys(StreamerCentral.instances).forEach((name) => delete StreamerCentral.instances[name]);
113+
});
114+
115+
['force_logout', 'notification', 'message', 'uiInteraction', 'subscriptions-changed', 'webdav', 'banners'].forEach((event) => {
116+
it(`should deny and not relay an arbitrary "${event}" event to other room members`, async () => {
117+
const emitSpy = sinon.spy(notifications.streamUser, 'emit');
118+
119+
const result = await writeAllowed(`room1/${event}`, { foo: 'bar' });
120+
121+
expect(result).to.equal(false);
122+
expect(findSubscriptionsExcludingUserStub.called).to.equal(false);
123+
expect(emitSpy.called).to.equal(false);
124+
});
125+
});
126+
127+
it('should not relay anything for a user not subscribed to the room', async () => {
128+
countByRoomIdAndUserIdStub.resolves(0);
129+
const emitSpy = sinon.spy(notifications.streamUser, 'emit');
130+
131+
const result = await writeAllowed('room1/video-conference', {
132+
action: 'call-start',
133+
params: { callId: '123', uid: 'victim', rid: 'room1' },
134+
});
135+
136+
expect(result).to.equal(false);
137+
expect(findSubscriptionsExcludingUserStub.called).to.equal(false);
138+
expect(emitSpy.called).to.equal(false);
139+
});
140+
141+
['video-conference', 'userData'].forEach((event) => {
142+
it(`should relay "${event}" event to other room members`, async () => {
143+
const emitSpy = sinon.spy(notifications.streamUser, 'emit');
144+
145+
const result = await writeAllowed(`room1/${event}`, { foo: 'bar' });
146+
147+
expect(result).to.equal(false);
148+
expect(findSubscriptionsExcludingUserStub.called).to.equal(true);
149+
expect(emitSpy.called).to.equal(true);
150+
});
151+
});
78152
});
79153
});

0 commit comments

Comments
 (0)