Skip to content

Commit

Permalink
fix: imported fixes (#33583)
Browse files Browse the repository at this point in the history
  • Loading branch information
Julio A. authored Oct 15, 2024
1 parent 5cc9ac5 commit e14fa89
Show file tree
Hide file tree
Showing 11 changed files with 133 additions and 8 deletions.
5 changes: 5 additions & 0 deletions .changeset/selfish-schools-leave.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)
6 changes: 6 additions & 0 deletions apps/meteor/app/api/server/v1/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type { FindOptions } from 'mongodb';
import _ from 'underscore';

import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
import { disableCustomScripts } from '../../../lib/server/functions/disableCustomScripts';
import { notifyOnSettingChanged, notifyOnSettingChangedById } from '../../../lib/server/lib/notifyListener';
import { SettingsEvents, settings } from '../../../settings/server';
import { setValue } from '../../../settings/server/raw';
Expand Down Expand Up @@ -173,6 +174,11 @@ API.v1.addRoute(
throw new Meteor.Error('error-id-param-not-provided', 'The parameter "id" is required');
}

// Disable custom scripts in cloud trials to prevent phishing campaigns
if (disableCustomScripts() && /^Custom_Script_/.test(this.urlParams._id)) {
return API.v1.unauthorized();
}

// allow special handling of particular setting types
const setting = await Settings.findOneNotHiddenById(this.urlParams._id);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,8 @@ export const createDiscussion = async (
if (!(await hasAtLeastOnePermissionAsync(userId, ['start-discussion', 'start-discussion-other-user'], prid))) {
throw new Meteor.Error('error-action-not-allowed', 'You are not allowed to create a discussion', { method: 'createDiscussion' });
}
const user = await Users.findOneById(userId);
const user = await Users.findOneById(userId, { projection: { services: 0 } });

if (!user) {
throw new Meteor.Error('error-invalid-user', 'Invalid user', {
method: 'createDiscussion',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,8 @@ export const sendFileMessage = async (
parseAttachmentsForE2EE: true,
},
): Promise<boolean> => {
const user = await Users.findOneById(userId);
const user = await Users.findOneById(userId, { projection: { services: 0 } });

if (!user) {
throw new Meteor.Error('error-invalid-user', 'Invalid user', {
method: 'sendFileMessage',
Expand Down
22 changes: 18 additions & 4 deletions apps/meteor/app/integrations/server/lib/triggerHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,20 @@ class RocketChatIntegrationHandler {
}

mapEventArgsToData(data, { event, message, room, owner, user }) {
/* The "services" field contains sensitive information such as
the user's password hash. To prevent this information from being
sent to the webhook, we're checking and removing it by destructuring
the user and owner objects while discarding the "services" field.
*/

const omitServicesField = (object) => {
const { services, ...objectWithoutServicesField } = object;
return objectWithoutServicesField;
};

const userWithoutServicesField = user?.services ? omitServicesField(user) : user;
const ownerWithoutServicesField = owner?.services ? omitServicesField(owner) : owner;

switch (event) {
case 'sendMessage':
data.channel_id = room._id;
Expand Down Expand Up @@ -215,7 +229,7 @@ class RocketChatIntegrationHandler {
data.user_id = message.u._id;
data.user_name = message.u.username;
data.text = message.msg;
data.user = user;
data.user = userWithoutServicesField;
data.room = room;
data.message = message;

Expand All @@ -233,7 +247,7 @@ class RocketChatIntegrationHandler {
data.timestamp = room.ts;
data.user_id = owner._id;
data.user_name = owner.username;
data.owner = owner;
data.owner = ownerWithoutServicesField;
data.room = room;
break;
case 'roomArchived':
Expand All @@ -244,7 +258,7 @@ class RocketChatIntegrationHandler {
data.channel_name = room.name;
data.user_id = user._id;
data.user_name = user.username;
data.user = user;
data.user = userWithoutServicesField;
data.room = room;

if (user.type === 'bot') {
Expand All @@ -255,7 +269,7 @@ class RocketChatIntegrationHandler {
data.timestamp = user.createdAt;
data.user_id = user._id;
data.user_name = user.username;
data.user = user;
data.user = userWithoutServicesField;

if (user.type === 'bot') {
data.bot = true;
Expand Down
14 changes: 14 additions & 0 deletions apps/meteor/app/lib/server/functions/disableCustomScripts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { License } from '@rocket.chat/license';

export const disableCustomScripts = () => {
const license = License.getLicense();

if (!license) {
return false;
}

const isCustomScriptDisabled = process.env.DISABLE_CUSTOM_SCRIPTS === 'true';
const isTrialLicense = license?.information.trial;

return isCustomScriptDisabled && isTrialLicense;
};
2 changes: 1 addition & 1 deletion apps/meteor/app/lib/server/methods/createChannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const createChannelMethod = async (
throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'createChannel' });
}

const user = await Users.findOneById(userId);
const user = await Users.findOneById(userId, { projection: { services: 0 } });
if (!user?.username) {
throw new Meteor.Error('error-invalid-user', 'Invalid user', { method: 'createChannel' });
}
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/app/lib/server/methods/createPrivateGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ Meteor.methods<ServerMethods>({
});
}

const user = await Users.findOneById(uid);
const user = await Users.findOneById(uid, { projection: { services: 0 } });
if (!user) {
throw new Meteor.Error('error-invalid-user', 'Invalid user', {
method: 'createPrivateGroup',
Expand Down
9 changes: 9 additions & 0 deletions apps/meteor/app/lib/server/methods/saveSetting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Meteor } from 'meteor/meteor';
import { twoFactorRequired } from '../../../2fa/server/twoFactorRequired';
import { getSettingPermissionId } from '../../../authorization/lib';
import { hasPermissionAsync, hasAllPermissionAsync } from '../../../authorization/server/functions/hasPermission';
import { disableCustomScripts } from '../functions/disableCustomScripts';
import { notifyOnSettingChanged } from '../lib/notifyListener';

declare module '@rocket.chat/ddp-client' {
Expand Down Expand Up @@ -39,6 +40,14 @@ Meteor.methods<ServerMethods>({
// Verify the _id passed in is a string.
check(_id, String);

// Disable custom scripts in cloud trials to prevent phishing campaigns
if (disableCustomScripts() && /^Custom_Script_/.test(_id)) {
throw new Meteor.Error('error-action-not-allowed', 'Editing settings is not allowed', {
method: 'saveSetting',
settingId: _id,
});
}

const setting = await Settings.findOneById(_id);

// Verify the value is what it should be
Expand Down
6 changes: 6 additions & 0 deletions apps/meteor/app/lib/server/methods/saveSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { twoFactorRequired } from '../../../2fa/server/twoFactorRequired';
import { getSettingPermissionId } from '../../../authorization/lib';
import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
import { settings } from '../../../settings/server';
import { disableCustomScripts } from '../functions/disableCustomScripts';
import { notifyOnSettingChangedById } from '../lib/notifyListener';

declare module '@rocket.chat/ddp-client' {
Expand Down Expand Up @@ -73,6 +74,11 @@ Meteor.methods<ServerMethods>({
return settingsNotAllowed.push(_id);
}

// Disable custom scripts in cloud trials to prevent phishing campaigns
if (disableCustomScripts() && /^Custom_Script_/.test(_id)) {
return settingsNotAllowed.push(_id);
}

const setting = await Settings.findOneById(_id);
// Verify the value is what it should be
switch (setting?.type) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { expect } from 'chai';
import proxyquire from 'proxyquire';
import sinon from 'sinon';

describe('disableCustomScripts', () => {
let mockLicense: sinon.SinonStubbedInstance<any>;
let disableCustomScripts: () => boolean;
let disableCustomScriptsVar: any;

beforeEach(() => {
disableCustomScriptsVar = process.env.DISABLE_CUSTOM_SCRIPTS;
mockLicense = {
getLicense: sinon.stub(),
};

disableCustomScripts = proxyquire('../../../../../../app/lib/server/functions/disableCustomScripts.ts', {
'@rocket.chat/license': { License: mockLicense },
}).disableCustomScripts;
});

afterEach(() => {
process.env.DISABLE_CUSTOM_SCRIPTS = disableCustomScriptsVar;
sinon.restore();
});

it('should return false when license is missing', () => {
mockLicense.getLicense.returns(null);

const result = disableCustomScripts();
expect(result).to.be.false;
});

it('should return false when DISABLE_CUSTOM_SCRIPTS is not true', () => {
mockLicense.getLicense.returns({
information: {
trial: true,
},
});

const result = disableCustomScripts();
expect(result).to.be.false;
});

it('should return false when license is not a trial', () => {
mockLicense.getLicense.returns({
information: {
trial: false,
},
});

process.env.DISABLE_CUSTOM_SCRIPTS = 'true';

const result = disableCustomScripts();
expect(result).to.be.false;
});

it('should return true when DISABLE_CUSTOM_SCRIPTS is true and license is a trial', () => {
mockLicense.getLicense.returns({
information: {
trial: true,
},
});

process.env.DISABLE_CUSTOM_SCRIPTS = 'true';

const result = disableCustomScripts();
expect(result).to.be.true;
});
});

0 comments on commit e14fa89

Please sign in to comment.