From e14fa89450540f0450877d30582703018f9d879c Mon Sep 17 00:00:00 2001
From: "Julio A." <52619625+julio-cfa@users.noreply.github.com>
Date: Wed, 16 Oct 2024 01:17:31 +0200
Subject: [PATCH] fix: imported fixes (#33583)
---
.changeset/selfish-schools-leave.md | 5 ++
apps/meteor/app/api/server/v1/settings.ts | 6 ++
.../server/methods/createDiscussion.ts | 3 +-
.../server/methods/sendFileMessage.ts | 3 +-
.../integrations/server/lib/triggerHandler.js | 22 ++++--
.../server/functions/disableCustomScripts.ts | 14 ++++
.../app/lib/server/methods/createChannel.ts | 2 +-
.../lib/server/methods/createPrivateGroup.ts | 2 +-
.../app/lib/server/methods/saveSetting.ts | 9 +++
.../app/lib/server/methods/saveSettings.ts | 6 ++
.../functions/disableCustomScripts.tests.ts | 69 +++++++++++++++++++
11 files changed, 133 insertions(+), 8 deletions(-)
create mode 100644 .changeset/selfish-schools-leave.md
create mode 100644 apps/meteor/app/lib/server/functions/disableCustomScripts.ts
create mode 100644 apps/meteor/tests/unit/app/lib/server/functions/disableCustomScripts.tests.ts
diff --git a/.changeset/selfish-schools-leave.md b/.changeset/selfish-schools-leave.md
new file mode 100644
index 00000000000..eacb88108a0
--- /dev/null
+++ b/.changeset/selfish-schools-leave.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/settings.ts b/apps/meteor/app/api/server/v1/settings.ts
index 574f4ee6419..94b25120295 100644
--- a/apps/meteor/app/api/server/v1/settings.ts
+++ b/apps/meteor/app/api/server/v1/settings.ts
@@ -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';
@@ -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);
diff --git a/apps/meteor/app/discussion/server/methods/createDiscussion.ts b/apps/meteor/app/discussion/server/methods/createDiscussion.ts
index 96e0bd84639..f2b4533489d 100644
--- a/apps/meteor/app/discussion/server/methods/createDiscussion.ts
+++ b/apps/meteor/app/discussion/server/methods/createDiscussion.ts
@@ -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',
diff --git a/apps/meteor/app/file-upload/server/methods/sendFileMessage.ts b/apps/meteor/app/file-upload/server/methods/sendFileMessage.ts
index 73dfd0216a7..8ed534c7015 100644
--- a/apps/meteor/app/file-upload/server/methods/sendFileMessage.ts
+++ b/apps/meteor/app/file-upload/server/methods/sendFileMessage.ts
@@ -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',
diff --git a/apps/meteor/app/integrations/server/lib/triggerHandler.js b/apps/meteor/app/integrations/server/lib/triggerHandler.js
index 7a2992f9151..a9e079a8382 100644
--- a/apps/meteor/app/integrations/server/lib/triggerHandler.js
+++ b/apps/meteor/app/integrations/server/lib/triggerHandler.js
@@ -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;
@@ -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;
@@ -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':
@@ -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') {
@@ -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;
diff --git a/apps/meteor/app/lib/server/functions/disableCustomScripts.ts b/apps/meteor/app/lib/server/functions/disableCustomScripts.ts
new file mode 100644
index 00000000000..0d1f1a2b320
--- /dev/null
+++ b/apps/meteor/app/lib/server/functions/disableCustomScripts.ts
@@ -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;
+};
diff --git a/apps/meteor/app/lib/server/methods/createChannel.ts b/apps/meteor/app/lib/server/methods/createChannel.ts
index a490fa1e3c4..8903583fb24 100644
--- a/apps/meteor/app/lib/server/methods/createChannel.ts
+++ b/apps/meteor/app/lib/server/methods/createChannel.ts
@@ -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' });
}
diff --git a/apps/meteor/app/lib/server/methods/createPrivateGroup.ts b/apps/meteor/app/lib/server/methods/createPrivateGroup.ts
index 3b92b260782..1efa98948eb 100644
--- a/apps/meteor/app/lib/server/methods/createPrivateGroup.ts
+++ b/apps/meteor/app/lib/server/methods/createPrivateGroup.ts
@@ -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',
diff --git a/apps/meteor/app/lib/server/methods/saveSetting.ts b/apps/meteor/app/lib/server/methods/saveSetting.ts
index 4f4d29b9fa2..61f5fbbd34c 100644
--- a/apps/meteor/app/lib/server/methods/saveSetting.ts
+++ b/apps/meteor/app/lib/server/methods/saveSetting.ts
@@ -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' {
@@ -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
diff --git a/apps/meteor/app/lib/server/methods/saveSettings.ts b/apps/meteor/app/lib/server/methods/saveSettings.ts
index dffa986b721..7a18bbc808d 100644
--- a/apps/meteor/app/lib/server/methods/saveSettings.ts
+++ b/apps/meteor/app/lib/server/methods/saveSettings.ts
@@ -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' {
@@ -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) {
diff --git a/apps/meteor/tests/unit/app/lib/server/functions/disableCustomScripts.tests.ts b/apps/meteor/tests/unit/app/lib/server/functions/disableCustomScripts.tests.ts
new file mode 100644
index 00000000000..a06abe7ac27
--- /dev/null
+++ b/apps/meteor/tests/unit/app/lib/server/functions/disableCustomScripts.tests.ts
@@ -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;
+ });
+});
--
GitLab