Skip to content

Commit

Permalink
fix: security hotfix (#33062) (#33069)
Browse files Browse the repository at this point in the history
Co-authored-by: Julio A. <[email protected]>
  • Loading branch information
dionisio-bot[bot] and julio-cfa authored Aug 16, 2024
1 parent ddae74c commit 83b0025
Show file tree
Hide file tree
Showing 21 changed files with 605 additions and 46 deletions.
5 changes: 5 additions & 0 deletions .changeset/perfect-dancers-grab.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: 8 additions & 2 deletions apps/meteor/app/api/server/v1/rooms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@ import { Media } from '@rocket.chat/core-services';
import type { IRoom, IUpload } from '@rocket.chat/core-typings';
import { Messages, Rooms, Users, Uploads } from '@rocket.chat/models';
import type { Notifications } from '@rocket.chat/rest-typings';
import { isGETRoomsNameExists, isRoomsImagesProps, isRoomsMuteUnmuteUserProps, isRoomsExportProps } from '@rocket.chat/rest-typings';
import {
isGETRoomsNameExists,
isRoomsCleanHistoryProps,
isRoomsImagesProps,
isRoomsMuteUnmuteUserProps,
isRoomsExportProps,
} from '@rocket.chat/rest-typings';
import { Meteor } from 'meteor/meteor';

import { isTruthy } from '../../../../lib/isTruthy';
Expand Down Expand Up @@ -355,7 +361,7 @@ API.v1.addRoute(

API.v1.addRoute(
'rooms.cleanHistory',
{ authRequired: true },
{ authRequired: true, validateParams: isRoomsCleanHistoryProps },
{
async post() {
const { _id } = await findRoomByIdOrName({ params: this.bodyParams });
Expand Down
6 changes: 6 additions & 0 deletions apps/meteor/app/api/server/v1/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,12 @@ API.v1.addRoute(
{ authRequired: false },
{
async post() {
const isPasswordResetEnabled = settings.get('Accounts_PasswordReset');

if (!isPasswordResetEnabled) {
return API.v1.failure('Password reset is not enabled');
}

const { email } = this.bodyParams;
if (!email) {
return API.v1.failure("The 'email' param is required");
Expand Down
12 changes: 9 additions & 3 deletions apps/meteor/app/livechat/imports/server/rest/sms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,18 @@ const normalizeLocationSharing = (payload: ServiceData) => {
// @ts-expect-error - this is an special endpoint that requires the return to not be wrapped as regular returns
API.v1.addRoute('livechat/sms-incoming/:service', {
async post() {
if (!(await OmnichannelIntegration.isConfiguredSmsService(this.urlParams.service))) {
const { service } = this.urlParams;
if (!(await OmnichannelIntegration.isConfiguredSmsService(service))) {
return API.v1.failure('Invalid service');
}

const smsDepartment = settings.get<string>('SMS_Default_Omnichannel_Department');
const SMSService = await OmnichannelIntegration.getSmsService(this.urlParams.service);
const SMSService = await OmnichannelIntegration.getSmsService(service);

if (!SMSService.validateRequest(this.request)) {
return API.v1.failure('Invalid request');
}

const sms = SMSService.parse(this.bodyParams);
const { department } = this.queryParams;
let targetDepartment = await defineDepartment(department || smsDepartment);
Expand All @@ -122,7 +128,7 @@ API.v1.addRoute('livechat/sms-incoming/:service', {
},
source: {
type: OmnichannelSourceType.SMS,
alias: this.urlParams.service,
alias: service,
},
};

Expand Down
3 changes: 2 additions & 1 deletion apps/meteor/client/views/admin/oauthApps/EditOauthApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
FieldRow,
FieldError,
FieldHint,
PasswordInput,
TextAreaInput,
ToggleSwitch,
FieldGroup,
Expand Down Expand Up @@ -136,7 +137,7 @@ const EditOauthApp = ({ onChange, data, ...props }: EditOauthAppProps): ReactEle
<Field>
<FieldLabel>{t('Client_Secret')}</FieldLabel>
<FieldRow>
<TextInput value={data.clientSecret} />
<PasswordInput value={data.clientSecret} />
</FieldRow>
</Field>
<Field>
Expand Down
46 changes: 36 additions & 10 deletions apps/meteor/ee/server/lib/audit/methods.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,18 @@ Meteor.methods<ServerMethods>({
check(startDate, Date);
check(endDate, Date);

const user = await Meteor.userAsync();
const user = (await Meteor.userAsync()) as IUser;
if (!user || !(await hasPermissionAsync(user._id, 'can-audit'))) {
throw new Meteor.Error('Not allowed');
}

const userFields = {
_id: user._id,
username: user.username,
...(user.name && { name: user.name }),
...(user.avatarETag && { avatarETag: user.avatarETag }),
};

const rooms: IRoom[] = await LivechatRooms.findByVisitorIdAndAgentId(visitor, agent, {
projection: { _id: 1 },
}).toArray();
Expand All @@ -118,7 +125,7 @@ Meteor.methods<ServerMethods>({
await AuditLog.insertOne({
ts: new Date(),
results: messages.length,
u: user,
u: userFields,
fields: { msg, users: usernames, rids, room: name, startDate, endDate, type, visitor, agent },
});

Expand All @@ -128,11 +135,18 @@ Meteor.methods<ServerMethods>({
check(startDate, Date);
check(endDate, Date);

const user = await Meteor.userAsync();
const user = (await Meteor.userAsync()) as IUser;
if (!user || !(await hasPermissionAsync(user._id, 'can-audit'))) {
throw new Meteor.Error('Not allowed');
}

const userFields = {
_id: user._id,
username: user.username,
...(user.name && { name: user.name }),
...(user.avatarETag && { avatarETag: user.avatarETag }),
};

let rids;
let name;

Expand Down Expand Up @@ -169,9 +183,10 @@ Meteor.methods<ServerMethods>({
await AuditLog.insertOne({
ts: new Date(),
results: messages.length,
u: user,
u: userFields,
fields: { msg, users: usernames, rids, room: name, startDate, endDate, type, visitor, agent },
});

updateCounter({ settingsId: 'Message_Auditing_Panel_Load_Count' });

return messages;
Expand All @@ -183,13 +198,24 @@ Meteor.methods<ServerMethods>({
if (!uid || !(await hasPermissionAsync(uid, 'can-audit-log'))) {
throw new Meteor.Error('Not allowed');
}
return AuditLog.find({
// 'u._id': userId,
ts: {
$gt: startDate,
$lt: endDate,
return AuditLog.find(
{
// 'u._id': userId,
ts: {
$gt: startDate,
$lt: endDate,
},
},
}).toArray();
{
projection: {
'u.services': 0,
'u.roles': 0,
'u.lastLogin': 0,
'u.statusConnection': 0,
'u.emails': 0,
},
},
).toArray();
},
});

Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/server/lib/ldap/Manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export class LDAPManager {

const { attribute: idAttribute, value: id } = uniqueId;
const username = this.slugifyUsername(ldapUser, usedUsername || id || '') || undefined;
const emails = this.getLdapEmails(ldapUser, username);
const emails = this.getLdapEmails(ldapUser, username).map((email) => email.trim());
const name = this.getLdapName(ldapUser) || undefined;

const userData: IImportUser = {
Expand Down
13 changes: 13 additions & 0 deletions apps/meteor/server/methods/reportMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { IMessage } from '@rocket.chat/core-typings';
import { ModerationReports, Rooms, Users, Messages } from '@rocket.chat/models';
import type { ServerMethods } from '@rocket.chat/ui-contexts';
import { check } from 'meteor/check';
import { DDPRateLimiter } from 'meteor/ddp-rate-limiter';
import { Meteor } from 'meteor/meteor';

import { canAccessRoomAsync } from '../../app/authorization/server/functions/canAccessRoom';
Expand Down Expand Up @@ -82,3 +83,15 @@ Meteor.methods<ServerMethods>({
return true;
},
});

DDPRateLimiter.addRule(
{
type: 'method',
name: 'reportMessage',
userId() {
return true;
},
},
5,
60000,
);
13 changes: 13 additions & 0 deletions apps/meteor/server/methods/sendConfirmationEmail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Users } from '@rocket.chat/models';
import type { ServerMethods } from '@rocket.chat/ui-contexts';
import { Accounts } from 'meteor/accounts-base';
import { check } from 'meteor/check';
import { DDPRateLimiter } from 'meteor/ddp-rate-limiter';
import { Meteor } from 'meteor/meteor';

import { methodDeprecationLogger } from '../../app/lib/server/lib/deprecationWarningLogger';
Expand Down Expand Up @@ -31,3 +32,15 @@ Meteor.methods<ServerMethods>({
}
},
});

DDPRateLimiter.addRule(
{
type: 'method',
name: 'sendConfirmationEmail',
userId() {
return true;
},
},
5,
60000,
);
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Base64 } from '@rocket.chat/base64';
import type { ISMSProvider, ServiceData, SMSProviderResult, SMSProviderResponse } from '@rocket.chat/core-typings';
import { serverFetch as fetch } from '@rocket.chat/server-fetch';
import type { Request } from 'express';

import { settings } from '../../../../app/settings/server';
import { SystemLogger } from '../../../lib/logger/system';
Expand Down Expand Up @@ -196,6 +197,10 @@ export class Mobex implements ISMSProvider {
};
}

validateRequest(_request: Request): boolean {
return true;
}

error(error: Error & { reason?: string }): SMSProviderResponse {
let message = '';
if (error.reason) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { api } from '@rocket.chat/core-services';
import type { ISMSProvider, ServiceData, SMSProviderResponse, SMSProviderResult } from '@rocket.chat/core-typings';
import { Users } from '@rocket.chat/models';
import type { Request } from 'express';
import filesize from 'filesize';
import twilio from 'twilio';

Expand Down Expand Up @@ -244,6 +245,31 @@ export class Twilio implements ISMSProvider {
};
}

isRequestFromTwilio(signature: string, requestBody: object): boolean {
const authToken = settings.get<string>('SMS_Twilio_authToken');
const siteUrl = settings.get<string>('Site_Url');

if (!authToken || !siteUrl) {
SystemLogger.error(`(Twilio) -> URL or Twilio token not configured.`);
return false;
}

const twilioUrl = siteUrl.endsWith('/')
? `${siteUrl}api/v1/livechat/sms-incoming/twilio`
: `${siteUrl}/api/v1/livechat/sms-incoming/twilio`;
return twilio.validateRequest(authToken, signature, twilioUrl, requestBody);
}

validateRequest(request: Request): boolean {
// We're not getting original twilio requests on CI :p
if (process.env.TEST_MODE === 'true') {
return true;
}
const twilioHeader = request.headers['x-twilio-signature'] || '';
const twilioSignature = Array.isArray(twilioHeader) ? twilioHeader[0] : twilioHeader;
return this.isRequestFromTwilio(twilioSignature, request.body);
}

error(error: Error & { reason?: string }): SMSProviderResponse {
let message = '';
if (error.reason) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { api } from '@rocket.chat/core-services';
import type { ISMSProvider, ServiceData, SMSProviderResponse } from '@rocket.chat/core-typings';
import { Users } from '@rocket.chat/models';
import { serverFetch as fetch } from '@rocket.chat/server-fetch';
import type { Request } from 'express';
import filesize from 'filesize';

import { settings } from '../../../../app/settings/server';
Expand Down Expand Up @@ -162,6 +163,10 @@ export class Voxtelesys implements ISMSProvider {
};
}

validateRequest(_request: Request): boolean {
return true;
}

error(error: Error & { reason?: string }): SMSProviderResponse {
let message = '';
if (error.reason) {
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/server/settings/message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const createMessageSettings = () =>
],
});

await this.add('Message_Attachments_Strip_Exif', false, {
await this.add('Message_Attachments_Strip_Exif', true, {
type: 'boolean',
public: true,
i18nDescription: 'Message_Attachments_Strip_ExifDescription',
Expand Down
41 changes: 30 additions & 11 deletions apps/meteor/tests/end-to-end/api/01-users.js
Original file line number Diff line number Diff line change
Expand Up @@ -2470,18 +2470,37 @@ describe('[Users]', function () {
});

describe('[/users.forgotPassword]', () => {
it('should return an error when "Accounts_PasswordReset" is disabled', (done) => {
void updateSetting('Accounts_PasswordReset', false).then(() => {
void request
.post(api('users.forgotPassword'))
.send({
email: adminEmail,
})
.expect('Content-Type', 'application/json')
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('error', 'Password reset is not enabled');
})
.end(done);
});
});

it('should send email to user (return success), when is a valid email', (done) => {
request
.post(api('users.forgotPassword'))
.send({
email: adminEmail,
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
})
.end(done);
void updateSetting('Accounts_PasswordReset', true).then(() => {
void request
.post(api('users.forgotPassword'))
.send({
email: adminEmail,
})
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
})
.end(done);
});
});

it('should not send email to user(return error), when is a invalid email', (done) => {
Expand Down
1 change: 0 additions & 1 deletion apps/meteor/tests/end-to-end/api/09-rooms.js
Original file line number Diff line number Diff line change
Expand Up @@ -1027,7 +1027,6 @@ describe('[Rooms]', function () {
.end(done);
});
});

describe('[/rooms.info]', () => {
let testChannel;
let testGroup;
Expand Down
Loading

0 comments on commit 83b0025

Please sign in to comment.