fix: imported fixes 04-04-26

.changeset/hot-spoons-heal.md

@@ -0,0 +1,5 @@
+---
+'@rocket.chat/meteor': patch
+---
+
+Disables SAML login when it is set to validate signatures without the proper configuration for it

.changeset/rich-doodles-grow.md

@@ -0,0 +1,9 @@
+---
+'@rocket.chat/model-typings': patch
+'@rocket.chat/core-typings': patch
+'@rocket.chat/models': patch
+'@rocket.chat/i18n': patch
+'@rocket.chat/meteor': patch
+---
+
+Security Hotfix (https://docs.rocket.chat/docs/security-fixes-and-updates)

apps/meteor/.mocharc.js

@@ -11,6 +11,7 @@ module.exports = {
 	exit: true,
 	spec: [
 		'server/lib/callbacks.spec.ts',
+		'server/lib/cas/*.spec.ts',
 		'server/lib/ldap/*.spec.ts',
 		'server/lib/ldap/**/*.spec.ts',
 		'server/lib/dataExport/**/*.spec.ts',

apps/meteor/app/file-upload/server/lib/FileUpload.ts

@@ -94,7 +94,7 @@ const defaults: Record<string, () => Partial<StoreOptions>> = {
 		return {
 			collection: UserDataFiles,
 			getPath(file: IUpload) {
-				return `${settings.get('uniqueID')}/uploads/userData/${file.userId}`;
+				return `${settings.get('uniqueID')}/uploads/userData/${file.userId}/${file._id}`;
 			},
 			onValidate: FileUpload.uploadsOnValidate,
 			async onRead(_fileId: string, file: IUpload, req: http.IncomingMessage, res: http.ServerResponse) {

apps/meteor/app/file-upload/server/methods/sendFileMessage.spec.ts

@@ -0,0 +1,111 @@
+import { expect } from 'chai';
+import { describe, it, beforeEach } from 'mocha';
+import proxyquire from 'proxyquire';
+import sinon from 'sinon';
+
+const findOneByIdAndUserIdAndRoomId = sinon.stub();
+const updateFileMetadata = sinon.stub().resolves();
+const getPath = sinon.stub().returns('/path/to/file.txt');
+const isImagePreviewSupported = sinon.stub().returns(false);
+const getFileExtension = sinon.stub().returns('txt');
+
+const { parseFileIntoMessageAttachments } = proxyquire.noCallThru().load('./sendFileMessage', {
+	'@rocket.chat/models': {
+		Uploads: { findOneByIdAndUserIdAndRoomId, updateFileMetadata },
+		Rooms: { findOneById: sinon.stub() },
+		Users: { findOneById: sinon.stub() },
+	},
+	'meteor/check': {
+		check: sinon.stub(),
+		Match: {
+			Maybe: sinon.stub(),
+			Optional: sinon.stub(),
+			ObjectIncluding: sinon.stub(),
+		},
+	},
+	'meteor/meteor': {
+		Meteor: {
+			Error: class Error extends global.Error {},
+			methods: sinon.stub(),
+		},
+	},
+	'../lib/FileUpload': {
+		FileUpload: { getPath },
+	},
+	'./isImagePreviewSupported': { isImagePreviewSupported },
+	'../../../../lib/utils/getFileExtension': { getFileExtension },
+	'../../../../server/lib/callbacks': { callbacks: { runAsync: sinon.stub() } },
+	'../../../../server/lib/logger/system': { SystemLogger: { error: sinon.stub() } },
+	'../../../authorization/server/functions/canAccessRoom': { canAccessRoomAsync: sinon.stub().resolves(true) },
+	'../../../lib/server/methods/sendMessage': { executeSendMessage: sinon.stub().resolves({}) },
+});
+
+describe('sendFileMessage - Mass Assignment & Type Pollution Prevention', () => {
+	const mockUser = { _id: 'user123' };
+	const roomId = 'room123';
+
+	beforeEach(() => {
+		findOneByIdAndUserIdAndRoomId.reset();
+		updateFileMetadata.reset();
+
+		findOneByIdAndUserIdAndRoomId.resolves({ _id: 'file123' });
+	});
+
+	it('should filter out invalid types, nulls, and malicious fields before updating the database', async () => {
+		const maliciousFilePayload = {
+			_id: 'file123',
+			name: null, // invalid type, must be ignored
+			type: 'text/plain',
+			size: 1024,
+			description: 12345, // invalid type, must be ignored
+			typeGroup: 'image', // only valid field
+			content: null, // invalid type, must be ignored
+			maliciousRoleAssignment: 'admin', // mass assignment, must be ignored
+			$set: { bypassSecurity: true }, // mongo injection, must be ignored
+		};
+
+		await parseFileIntoMessageAttachments(maliciousFilePayload as any, roomId, mockUser as any);
+
+		expect(updateFileMetadata.calledOnce).to.equal(true);
+
+		const [fileId, userId, safeMetadata] = updateFileMetadata.getCall(0).args;
+
+		expect(fileId).to.equal('file123');
+		expect(userId).to.equal('user123');
+
+		expect(safeMetadata).to.deep.equal({
+			typeGroup: 'image',
+		});
+	});
+
+	it('should pass valid fields correctly to the database', async () => {
+		const validFilePayload = {
+			_id: 'file123',
+			name: 'picture.jpg',
+			type: 'image/jpeg',
+			size: 2048,
+			description: 'Description',
+			typeGroup: 'image',
+			content: {
+				algorithm: 'rc.v1.aes-sha2',
+				ciphertext: 'test',
+			},
+		};
+
+		await parseFileIntoMessageAttachments(validFilePayload as any, roomId, mockUser as any);
+
+		expect(updateFileMetadata.calledOnce).to.equal(true);
+
+		const [, , safeMetadata] = updateFileMetadata.getCall(0).args;
+
+		expect(safeMetadata).to.deep.equal({
+			name: 'picture.jpg',
+			description: 'Description',
+			typeGroup: 'image',
+			content: {
+				algorithm: 'rc.v1.aes-sha2',
+				ciphertext: 'test',
+			},
+		});
+	});
+});

apps/meteor/app/file-upload/server/methods/sendFileMessage.ts

@@ -15,7 +15,6 @@ import { Meteor } from 'meteor/meteor';
 
 import { isImagePreviewSupported } from './isImagePreviewSupported';
 import { getFileExtension } from '../../../../lib/utils/getFileExtension';
-import { omit } from '../../../../lib/utils/omit';
 import { callbacks } from '../../../../server/lib/callbacks';
 import { SystemLogger } from '../../../../server/lib/logger/system';
 import { canAccessRoomAsync } from '../../../authorization/server/functions/canAccessRoom';
@@ -46,7 +45,14 @@ export const parseFileIntoMessageAttachments = async (
 		});
 	}
 
-	await Uploads.updateFileComplete(file._id, user._id, omit(file, '_id'));
+	const safeMetadata = {
+		...(typeof file.name === 'string' && { name: file.name }),
+		...(typeof file.description === 'string' && { description: file.description }),
+		...(typeof file.typeGroup === 'string' && { typeGroup: file.typeGroup }),
+		...(file.content && typeof file.content === 'object' && { content: file.content }),
+	};
+
+	await Uploads.updateFileMetadata(file._id, user._id, safeMetadata);
 
 	const fileUrl = FileUpload.getPath(`${file._id}/${encodeURI(file.name || '')}`);
 

apps/meteor/app/meteor-accounts-saml/server/definition/IServiceProviderOptions.ts

@@ -14,6 +14,8 @@ export interface IServiceProviderOptions {
 	defaultUserRole: string;
 	allowedClockDrift: number;
 	signatureValidationType: string;
+	validateLogoutRequestSignature: boolean;
+	validateLogoutResponseSignature: boolean;
 	identifierFormat: string;
 	nameIDPolicyTemplate: string;
 	authnContextTemplate: string;

apps/meteor/app/meteor-accounts-saml/server/definition/SAMLEnvelope.ts

@@ -0,0 +1,43 @@
+// Other binding values are not supported: HTTP-Artifact, URI, SOAP, PAOS
+export type SAMLBinding = 'HTTP-Redirect' | 'HTTP-POST';
+
+export type SAMLDocumentType = 'SAMLRequest' | 'SAMLResponse';
+
+export type SAMLBaseEnvelope<T extends SAMLDocumentType = SAMLDocumentType> = {
+	binding: SAMLBinding;
+	/**
+	 * encodedDocument is the document in the exact format we received it
+	 * */
+	encodedDocument: string;
+	/**
+	 * decodedDocument is the raw XML file; processed according to the binding specification
+	 * */
+	decodedDocument: string;
+	type: T;
+	relayState?: string;
+};
+
+/**
+ * HTTP-Redirect envelope
+ * For this binding, the document must be a Deflated XML file that is then base64 encoded
+ **/
+export type SAMLRedirectEnvelope<T extends SAMLDocumentType = SAMLDocumentType> = SAMLBaseEnvelope<T> & {
+	binding: 'HTTP-Redirect';
+	sigAlg?: string;
+	signature?: string;
+
+	/**
+	 * signedContent is the complete string value that must be matched by the signature
+	 * */
+	signedContent?: string;
+};
+
+/**
+ * HTTP-POST envelope
+ * For this binding, the document must be a regular XML file that is base64 encoded
+ **/
+export type SAMLPOSTEnvelope<T extends SAMLDocumentType = SAMLDocumentType> = SAMLBaseEnvelope<T> & {
+	binding: 'HTTP-POST';
+};
+
+export type SAMLEnvelope<T extends SAMLDocumentType = SAMLDocumentType> = SAMLRedirectEnvelope<T> | SAMLPOSTEnvelope<T>;

apps/meteor/app/meteor-accounts-saml/server/lib/SAML.ts

@@ -9,6 +9,7 @@ import { Meteor } from 'meteor/meteor';
 
 import { SAMLServiceProvider } from './ServiceProvider';
 import { SAMLUtils } from './Utils';
+import { getSAMLEnvelope } from './getSAMLEnvelope';
 import { ensureArray } from '../../../../lib/utils/arrayUtils';
 import { SystemLogger } from '../../../../server/lib/logger/system';
 import { addUserToRoom } from '../../../lib/server/functions/addUserToRoom';
@@ -283,11 +284,21 @@ export class SAML {
 	}
 
 	private static async processLogoutRequest(req: IIncomingMessage, res: ServerResponse, service: IServiceProviderOptions): Promise<void> {
+		const errorHandler = (err: unknown) => {
+			SystemLogger.error({ err });
+			throw new Meteor.Error('Unable to Validate Logout Request');
+		};
+
+		const envelope = await getSAMLEnvelope(req, 'SAMLRequest', 'HTTP-Redirect').catch(errorHandler);
+		if (!envelope) {
+			errorHandler('No envelope found for SAML Logout Request');
+			return;
+		}
+
 		const serviceProvider = new SAMLServiceProvider(service);
-		await serviceProvider.validateLogoutRequest(req.query.SAMLRequest, async (err, result) => {
+		await serviceProvider.validateLogoutRequest(envelope, async (err, result) => {
 			if (err) {
-				SystemLogger.error({ err });
-				throw new Meteor.Error('Unable to Validate Logout Request');
+				errorHandler(err);
 			}
 
 			if (!result?.nameID || !result?.idpSession) {
@@ -350,15 +361,16 @@ export class SAML {
 	}
 
 	private static async processLogoutResponse(req: IIncomingMessage, res: ServerResponse, service: IServiceProviderOptions): Promise<void> {
-		if (!req.query.SAMLResponse) {
+		const envelope = await getSAMLEnvelope(req, 'SAMLResponse', 'HTTP-Redirect');
+		if (!envelope) {
 			SAMLUtils.error({ msg: 'Invalid LogoutResponse received: missing SAMLResponse parameter.', query: req.query });
 			throw new Error('Invalid LogoutResponse received.');
 		}
 
 		const serviceProvider = new SAMLServiceProvider(service);
-		await serviceProvider.validateLogoutResponse(req.query.SAMLResponse, async (err, inResponseTo) => {
+		await serviceProvider.validateLogoutResponse(envelope, async (err, inResponseTo) => {
 			if (err) {
-				return;
+				throw new Error('Logout Response Validation failed');
 			}
 
 			if (!inResponseTo) {
@@ -460,15 +472,29 @@ export class SAML {
 		res.end();
 	}
 
-	private static processValidateAction(
+	private static async processValidateAction(
 		req: IIncomingMessage,
 		res: ServerResponse,
 		service: IServiceProviderOptions,
 		_samlObject: ISAMLAction,
-	): void {
+	): Promise<void> {
+		const redirect = (url?: string) => {
+			res.writeHead(302, {
+				Location: url ?? Meteor.absoluteUrl(),
+			});
+			res.end();
+		};
+
+		const envelope = await getSAMLEnvelope(req, 'SAMLResponse', 'HTTP-POST').catch((err) => SAMLUtils.error(err));
+
+		if (!envelope) {
+			SAMLUtils.error({ msg: 'No envelope found for SAML Response' });
+			return redirect();
+		}
+
 		const serviceProvider = new SAMLServiceProvider(service);
-		SAMLUtils.relayState = req.body.RelayState;
-		serviceProvider.validateResponse(req.body.SAMLResponse, async (err, profile /* , loggedOut*/) => {
+		SAMLUtils.relayState = envelope.relayState ?? null;
+		serviceProvider.validateResponse(envelope, async (err, profile /* , loggedOut*/) => {
 			try {
 				if (err) {
 					SAMLUtils.error(err);
@@ -490,16 +516,10 @@ export class SAML {
 
 				await this.storeCredential(credentialToken, loginResult);
 				const url = Meteor.absoluteUrl(SAMLUtils.getValidationActionRedirectPath(credentialToken));
-				res.writeHead(302, {
-					Location: url,
-				});
-				res.end();
+				redirect(url);
 			} catch (err) {
 				SAMLUtils.error({ err });
-				res.writeHead(302, {
-					Location: Meteor.absoluteUrl(),
-				});
-				res.end();
+				redirect();
 			}
 		});
 	}