From ce21727e44d34560193cc6ec1d376643ab5cdd49 Mon Sep 17 00:00:00 2001 From: "dionisio-bot[bot]" <117394943+dionisio-bot[bot]@users.noreply.github.com> Date: Mon, 29 Jul 2024 10:40:31 -0300 Subject: [PATCH] fix: imported fixes (#32894) (#32919) Co-authored-by: Diego Sampaio Co-authored-by: Julio A. <52619625+julio-cfa@users.noreply.github.com> --- .changeset/hungry-jars-lay.md | 5 + apps/meteor/app/2fa/server/methods/enable.ts | 4 + apps/meteor/app/api/server/v1/chat.ts | 5 + apps/meteor/app/api/server/v1/users.ts | 21 +- .../lib/server/functions/checkUrlForSsrf.ts | 100 ++++++++++ .../app/lib/server/functions/saveUser.js | 7 + .../app/lib/server/functions/setUserAvatar.ts | 12 +- .../lib/server/functions/validateNameChars.ts | 21 ++ .../app/lib/server/methods/sendMessage.ts | 7 + .../app/livechat/imports/server/rest/sms.ts | 8 +- apps/meteor/server/routes/avatar/room.js | 10 +- apps/meteor/server/routes/avatar/user.js | 8 +- apps/meteor/tests/end-to-end/api/01-users.js | 32 ++++ apps/meteor/tests/end-to-end/api/05-chat.js | 22 +++ .../meteor/tests/end-to-end/api/24-methods.js | 180 ++++++++++++++++++ .../end-to-end/api/methods/2fa-enable.ts | 158 +++++++++++++++ .../lib/server/lib/checkUrlForSsrf.tests.ts | 52 +++++ .../lib/server/lib/validateNameChars.tests.ts | 56 ++++++ packages/i18n/src/locales/en.i18n.json | 1 + packages/i18n/src/locales/pt-BR.i18n.json | 1 + packages/i18n/src/locales/pt.i18n.json | 1 + .../web-ui-registration/src/RegisterForm.tsx | 5 +- 22 files changed, 708 insertions(+), 8 deletions(-) create mode 100644 .changeset/hungry-jars-lay.md create mode 100644 apps/meteor/app/lib/server/functions/checkUrlForSsrf.ts create mode 100644 apps/meteor/app/lib/server/functions/validateNameChars.ts create mode 100644 apps/meteor/tests/end-to-end/api/methods/2fa-enable.ts create mode 100644 apps/meteor/tests/unit/app/lib/server/lib/checkUrlForSsrf.tests.ts create mode 100644 apps/meteor/tests/unit/app/lib/server/lib/validateNameChars.tests.ts diff --git a/.changeset/hungry-jars-lay.md b/.changeset/hungry-jars-lay.md new file mode 100644 index 000000000000..eacb88108a0f --- /dev/null +++ b/.changeset/hungry-jars-lay.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/2fa/server/methods/enable.ts b/apps/meteor/app/2fa/server/methods/enable.ts index 3b9f35dfcd9d..6b786c0743e9 100644 --- a/apps/meteor/app/2fa/server/methods/enable.ts +++ b/apps/meteor/app/2fa/server/methods/enable.ts @@ -34,6 +34,10 @@ Meteor.methods({ }); } + if (user.services?.totp?.enabled) { + throw new Meteor.Error('error-2fa-already-enabled'); + } + const secret = TOTP.generateSecret(); await Users.disable2FAAndSetTempSecretByUserId(userId, secret.base32); diff --git a/apps/meteor/app/api/server/v1/chat.ts b/apps/meteor/app/api/server/v1/chat.ts index c482e3bb784d..3ccc9caeafa0 100644 --- a/apps/meteor/app/api/server/v1/chat.ts +++ b/apps/meteor/app/api/server/v1/chat.ts @@ -18,6 +18,7 @@ import { executeUpdateMessage } from '../../../lib/server/methods/updateMessage' import { OEmbed } from '../../../oembed/server/server'; import { executeSetReaction } from '../../../reactions/server/setReaction'; import { settings } from '../../../settings/server'; +import { MessageTypes } from '../../../ui-utils/server'; import { normalizeMessagesForUser } from '../../../utils/server/lib/normalizeMessagesForUser'; import { API } from '../api'; import { getPaginationItems } from '../helpers/getPaginationItems'; @@ -217,6 +218,10 @@ API.v1.addRoute( throw new Meteor.Error('error-invalid-params', 'The "message" parameter must be provided.'); } + if (MessageTypes.isSystemMessage(this.bodyParams.message)) { + throw new Error("Cannot send system messages using 'chat.sendMessage'"); + } + const sent = await executeSendMessage(this.userId, this.bodyParams.message as Pick, this.bodyParams.previewUrls); const [message] = await normalizeMessagesForUser([sent], this.userId); diff --git a/apps/meteor/app/api/server/v1/users.ts b/apps/meteor/app/api/server/v1/users.ts index ccca23f8ea82..6dfbd4e139cb 100644 --- a/apps/meteor/app/api/server/v1/users.ts +++ b/apps/meteor/app/api/server/v1/users.ts @@ -43,6 +43,7 @@ import { setStatusText } from '../../../lib/server/functions/setStatusText'; import { setUserAvatar } from '../../../lib/server/functions/setUserAvatar'; import { setUsernameWithValidation } from '../../../lib/server/functions/setUsername'; import { validateCustomFields } from '../../../lib/server/functions/validateCustomFields'; +import { validateNameChars } from '../../../lib/server/functions/validateNameChars'; import { generateAccessToken } from '../../../lib/server/methods/createToken'; import { settings } from '../../../settings/server'; import { getURL } from '../../../utils/server/getURL'; @@ -93,6 +94,10 @@ API.v1.addRoute( async post() { const userData = { _id: this.bodyParams.userId, ...this.bodyParams.data }; + if (userData.name && !validateNameChars(userData.name)) { + return API.v1.failure('Name contains invalid characters'); + } + await saveUser(this.userId, userData); if (this.bodyParams.data.customFields) { @@ -137,6 +142,10 @@ API.v1.addRoute( typedPassword: this.bodyParams.data.currentPassword, }; + if (userData.realname && !validateNameChars(userData.realname)) { + return API.v1.failure('Name contains invalid characters'); + } + // saveUserProfile now uses the default two factor authentication procedures, so we need to provide that const twoFactorOptions = !userData.typedPassword ? null @@ -279,6 +288,10 @@ API.v1.addRoute( this.bodyParams.joinDefaultChannels = true; } + if (this.bodyParams.name && !validateNameChars(this.bodyParams.name)) { + return API.v1.failure('Name contains invalid characters'); + } + if (this.bodyParams.customFields) { validateCustomFields(this.bodyParams.customFields); } @@ -625,16 +638,20 @@ API.v1.addRoute( }, { async post() { + const { secret: secretURL, ...params } = this.bodyParams; + if (this.userId) { return API.v1.failure('Logged in users can not register again.'); } + if (params.name && !validateNameChars(params.name)) { + return API.v1.failure('Name contains invalid characters'); + } + if (!(await checkUsernameAvailability(this.bodyParams.username))) { return API.v1.failure('Username is already in use'); } - const { secret: secretURL, ...params } = this.bodyParams; - if (this.bodyParams.customFields) { try { await validateCustomFields(this.bodyParams.customFields); diff --git a/apps/meteor/app/lib/server/functions/checkUrlForSsrf.ts b/apps/meteor/app/lib/server/functions/checkUrlForSsrf.ts new file mode 100644 index 000000000000..c90065d7ad8f --- /dev/null +++ b/apps/meteor/app/lib/server/functions/checkUrlForSsrf.ts @@ -0,0 +1,100 @@ +import { lookup } from 'dns'; + +// https://en.wikipedia.org/wiki/Reserved_IP_addresses + Alibaba Metadata IP +const ranges: string[] = [ + '0.0.0.0/8', + '10.0.0.0/8', + '100.64.0.0/10', + '127.0.0.0/8', + '169.254.0.0/16', + '172.16.0.0/12', + '192.0.0.0/24', + '192.0.2.0/24', + '192.88.99.0/24', + '192.168.0.0/16', + '198.18.0.0/15', + '198.51.100.0/24', + '203.0.113.0/24', + '224.0.0.0/4', + '240.0.0.0/4', + '255.255.255.255', + '100.100.100.200/32', +]; + +export const nslookup = async (hostname: string): Promise => { + return new Promise((resolve, reject) => { + lookup(hostname, (error, address) => { + if (error) { + reject(error); + } else { + resolve(address); + } + }); + }); +}; + +export const ipToLong = (ip: string): number => { + return ip.split('.').reduce((acc, octet) => (acc << 8) + parseInt(octet, 10), 0) >>> 0; +}; + +export const isIpInRange = (ip: string, range: string): boolean => { + const [rangeIp, subnet] = range.split('/'); + const ipLong = ipToLong(ip); + const rangeIpLong = ipToLong(rangeIp); + const mask = ~(2 ** (32 - Number(subnet)) - 1); + return (ipLong & mask) === (rangeIpLong & mask); +}; + +export const isIpInAnyRange = (ip: string): boolean => ranges.some((range) => isIpInRange(ip, range)); + +export const isValidIPv4 = (ip: string): boolean => { + const octets = ip.split('.'); + if (octets.length !== 4) return false; + return octets.every((octet) => { + const num = Number(octet); + return num >= 0 && num <= 255 && octet === num.toString(); + }); +}; + +export const isValidDomain = (domain: string): boolean => { + const domainPattern = /^(?!-)(?!.*--)[A-Za-z0-9-]{1,63}(? => { + if (!(url.startsWith('http://') || url.startsWith('https://'))) { + return false; + } + + const [, address] = url.split('://'); + const ipOrDomain = address.includes('/') ? address.split('/')[0] : address; + + if (!(isValidIPv4(ipOrDomain) || isValidDomain(ipOrDomain))) { + return false; + } + + if (isValidIPv4(ipOrDomain) && isIpInAnyRange(ipOrDomain)) { + return false; + } + + if (isValidDomain(ipOrDomain) && /metadata.google.internal/.test(ipOrDomain.toLowerCase())) { + return false; + } + + if (isValidDomain(ipOrDomain)) { + try { + const ipAddress = await nslookup(ipOrDomain); + if (isIpInAnyRange(ipAddress)) { + return false; + } + } catch (error) { + console.log(error); + return false; + } + } + + return true; +}; diff --git a/apps/meteor/app/lib/server/functions/saveUser.js b/apps/meteor/app/lib/server/functions/saveUser.js index 3a2808b4171c..027c890aab66 100644 --- a/apps/meteor/app/lib/server/functions/saveUser.js +++ b/apps/meteor/app/lib/server/functions/saveUser.js @@ -68,6 +68,13 @@ async function _sendUserEmail(subject, html, userData) { async function validateUserData(userId, userData) { const existingRoles = _.pluck(await getRoles(), '_id'); + if (userData.verified && userData._id && userId === userData._id) { + throw new Meteor.Error('error-action-not-allowed', 'Editing email verification is not allowed', { + method: 'insertOrUpdateUser', + action: 'Editing_user', + }); + } + if (userData._id && userId !== userData._id && !(await hasPermissionAsync(userId, 'edit-other-user-info'))) { throw new Meteor.Error('error-action-not-allowed', 'Editing user is not allowed', { method: 'insertOrUpdateUser', diff --git a/apps/meteor/app/lib/server/functions/setUserAvatar.ts b/apps/meteor/app/lib/server/functions/setUserAvatar.ts index b46f0ff8cd50..13ccd2de6954 100644 --- a/apps/meteor/app/lib/server/functions/setUserAvatar.ts +++ b/apps/meteor/app/lib/server/functions/setUserAvatar.ts @@ -10,6 +10,7 @@ import { hasPermissionAsync } from '../../../authorization/server/functions/hasP import { FileUpload } from '../../../file-upload/server'; import { RocketChatFile } from '../../../file/server'; import { settings } from '../../../settings/server'; +import { checkUrlForSsrf } from './checkUrlForSsrf'; export const setAvatarFromServiceWithValidation = async ( userId: string, @@ -88,8 +89,17 @@ export async function setUserAvatar( const { buffer, type } = await (async (): Promise<{ buffer: Buffer; type: string }> => { if (service === 'url' && typeof dataURI === 'string') { let response: Response; + + const isSsrfSafe = await checkUrlForSsrf(dataURI); + if (!isSsrfSafe) { + throw new Meteor.Error('error-avatar-invalid-url', `Invalid avatar URL: ${encodeURI(dataURI)}`, { + function: 'setUserAvatar', + url: dataURI, + }); + } + try { - response = await fetch(dataURI); + response = await fetch(dataURI, { redirect: 'error' }); } catch (e) { SystemLogger.info(`Not a valid response, from the avatar url: ${encodeURI(dataURI)}`); throw new Meteor.Error('error-avatar-invalid-url', `Invalid avatar URL: ${encodeURI(dataURI)}`, { diff --git a/apps/meteor/app/lib/server/functions/validateNameChars.ts b/apps/meteor/app/lib/server/functions/validateNameChars.ts new file mode 100644 index 000000000000..07330c66b762 --- /dev/null +++ b/apps/meteor/app/lib/server/functions/validateNameChars.ts @@ -0,0 +1,21 @@ +export const validateNameChars = (name: string | undefined): boolean => { + if (typeof name !== 'string') { + return false; + } + + const invalidChars = /[<>\\/]/; + if (invalidChars.test(name)) { + return false; + } + + try { + const decodedName = decodeURI(name); + if (invalidChars.test(decodedName)) { + return false; + } + } catch (err) { + return false; + } + + return true; +}; diff --git a/apps/meteor/app/lib/server/methods/sendMessage.ts b/apps/meteor/app/lib/server/methods/sendMessage.ts index e12ebc2d47e9..c0837af6aa29 100644 --- a/apps/meteor/app/lib/server/methods/sendMessage.ts +++ b/apps/meteor/app/lib/server/methods/sendMessage.ts @@ -12,6 +12,7 @@ import { canSendMessageAsync } from '../../../authorization/server/functions/can import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission'; import { metrics } from '../../../metrics/server'; import { settings } from '../../../settings/server'; +import { MessageTypes } from '../../../ui-utils/server'; import { sendMessage } from '../functions/sendMessage'; import { RateLimiter } from '../lib'; @@ -78,6 +79,8 @@ export async function executeSendMessage(uid: IUser['_id'], message: AtLeast({ }); } + if (MessageTypes.isSystemMessage(message)) { + throw new Error("Cannot send system messages using 'sendMessage'"); + } + try { return await executeSendMessage(uid, message, previewUrls); } catch (error: any) { diff --git a/apps/meteor/app/livechat/imports/server/rest/sms.ts b/apps/meteor/app/livechat/imports/server/rest/sms.ts index 2551b79cc425..f6502b70f68a 100644 --- a/apps/meteor/app/livechat/imports/server/rest/sms.ts +++ b/apps/meteor/app/livechat/imports/server/rest/sms.ts @@ -17,6 +17,7 @@ import { Meteor } from 'meteor/meteor'; import { getFileExtension } from '../../../../../lib/utils/getFileExtension'; import { API } from '../../../../api/server'; import { FileUpload } from '../../../../file-upload/server'; +import { checkUrlForSsrf } from '../../../../lib/server/functions/checkUrlForSsrf'; import { settings } from '../../../../settings/server'; import type { ILivechatMessage } from '../../../server/lib/LivechatTyped'; import { Livechat as LivechatTyped } from '../../../server/lib/LivechatTyped'; @@ -24,7 +25,12 @@ import { Livechat as LivechatTyped } from '../../../server/lib/LivechatTyped'; const logger = new Logger('SMS'); const getUploadFile = async (details: Omit, fileUrl: string) => { - const response = await fetch(fileUrl); + const isSsrfSafe = await checkUrlForSsrf(fileUrl); + if (!isSsrfSafe) { + throw new Meteor.Error('error-invalid-url', 'Invalid URL'); + } + + const response = await fetch(fileUrl, { redirect: 'error' }); const content = Buffer.from(await response.arrayBuffer()); diff --git a/apps/meteor/server/routes/avatar/room.js b/apps/meteor/server/routes/avatar/room.js index c47e58b48d0b..3482253d57d8 100644 --- a/apps/meteor/server/routes/avatar/room.js +++ b/apps/meteor/server/routes/avatar/room.js @@ -5,6 +5,9 @@ import { FileUpload } from '../../../app/file-upload/server'; import { roomCoordinator } from '../../lib/rooms/roomCoordinator'; import { renderSVGLetters, serveAvatar, wasFallbackModified, setCacheAndDispositionHeaders } from './utils'; +const MAX_ROOM_SVG_AVATAR_SIZE = 1024; +const MIN_ROOM_SVG_AVATAR_SIZE = 16; + const cookie = new Cookies(); const getRoomAvatar = async (roomId) => { const room = await Rooms.findOneById(roomId, { projection: { t: 1, prid: 1, name: 1, fname: 1, federated: 1 } }); @@ -64,7 +67,12 @@ export const roomAvatar = async function (req, res /* , next*/) { return; } - const svg = renderSVGLetters(roomName, req.query.size && parseInt(req.query.size)); + let avatarSize = req.query.size && parseInt(req.query.size); + if (avatarSize) { + avatarSize = Math.min(Math.max(avatarSize, MIN_ROOM_SVG_AVATAR_SIZE), MAX_ROOM_SVG_AVATAR_SIZE); + } + + const svg = renderSVGLetters(roomName, avatarSize); return serveAvatar(svg, req.query.format, res); }; diff --git a/apps/meteor/server/routes/avatar/user.js b/apps/meteor/server/routes/avatar/user.js index 7997a91d95a4..0d86bc4a08cf 100644 --- a/apps/meteor/server/routes/avatar/user.js +++ b/apps/meteor/server/routes/avatar/user.js @@ -4,6 +4,9 @@ import { FileUpload } from '../../../app/file-upload/server'; import { settings } from '../../../app/settings/server'; import { renderSVGLetters, serveAvatar, wasFallbackModified, setCacheAndDispositionHeaders } from './utils'; +const MAX_USER_SVG_AVATAR_SIZE = 1024; +const MIN_USER_SVG_AVATAR_SIZE = 16; + // request /avatar/@name forces returning the svg export const userAvatar = async function (req, res) { const requestUsername = decodeURIComponent(req.url.substr(1).replace(/\?.*$/, '')); @@ -14,7 +17,10 @@ export const userAvatar = async function (req, res) { return; } - const avatarSize = req.query.size && parseInt(req.query.size); + let avatarSize = req.query.size && parseInt(req.query.size); + if (avatarSize) { + avatarSize = Math.min(Math.max(avatarSize, MIN_USER_SVG_AVATAR_SIZE), MAX_USER_SVG_AVATAR_SIZE); + } setCacheAndDispositionHeaders(req, res); diff --git a/apps/meteor/tests/end-to-end/api/01-users.js b/apps/meteor/tests/end-to-end/api/01-users.js index cbc51204ae9f..44f9a370ed5d 100644 --- a/apps/meteor/tests/end-to-end/api/01-users.js +++ b/apps/meteor/tests/end-to-end/api/01-users.js @@ -456,6 +456,23 @@ describe('[Users]', function () { }) .end(done); }); + it("should return an error when registering a user's name with invalid characters: >, <, /, or \\", (done) => { + request + .post(api('users.register')) + .send({ + email, + name: '', + username, + pass: 'test', + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + expect(res.body).to.have.property('error').and.to.be.equal('Name contains invalid characters'); + }) + .end(done); + }); }); describe('[/users.info]', () => { @@ -1155,6 +1172,21 @@ describe('[Users]', function () { }); }); }); + it('should prevent users from passing server-side request forgery (SSRF) payloads as avatarUrl', (done) => { + request + .post(api('users.setAvatar')) + .set(credentials) + .send({ + userId: userCredentials['X-User-Id'], + avatarUrl: 'http://169.254.169.254/', + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + }) + .end(done); + }); }); describe('[/users.resetAvatar]', () => { diff --git a/apps/meteor/tests/end-to-end/api/05-chat.js b/apps/meteor/tests/end-to-end/api/05-chat.js index fe02112c2811..aa6f77cb7a00 100644 --- a/apps/meteor/tests/end-to-end/api/05-chat.js +++ b/apps/meteor/tests/end-to-end/api/05-chat.js @@ -1,3 +1,4 @@ +import { Random } from '@rocket.chat/random'; import { expect } from 'chai'; import { after, before, beforeEach, describe, it } from 'mocha'; @@ -1103,6 +1104,27 @@ describe('[Chat]', function () { .end(done); }); + it('should fail if message is a system message', () => { + const msgId = Random.id(); + return request + .post(api('chat.sendMessage')) + .set(credentials) + .send({ + message: { + _id: msgId, + rid: 'GENERAL', + msg: 'xss', + t: 'subscription-role-added', + role: '

XSS', + }, + }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + }); + }); + describe('customFields', () => { async function testMessageSending({ customFields, testCb, statusCode }) { await request diff --git a/apps/meteor/tests/end-to-end/api/24-methods.js b/apps/meteor/tests/end-to-end/api/24-methods.js index b9705aa599e1..aabd24e04119 100644 --- a/apps/meteor/tests/end-to-end/api/24-methods.js +++ b/apps/meteor/tests/end-to-end/api/24-methods.js @@ -1,3 +1,4 @@ +import { Random } from '@rocket.chat/random'; import { expect } from 'chai'; import { after, before, beforeEach, describe, it } from 'mocha'; @@ -1978,6 +1979,46 @@ describe('Meteor.methods', function () { }) .end(done); }); + + it('should not send message if it is a system message', async () => { + const msgId = Random.id(); + await request + .post(methodCall('sendMessage')) + .set(credentials) + .send({ + message: JSON.stringify({ + method: 'sendMessage', + params: [ + { + _id: msgId, + rid: 'GENERAL', + msg: 'xss', + t: 'subscription-role-added', + role: '

XSS', + }, + ], + id: 1000, + msg: 'method', + }), + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + const data = JSON.parse(res.body.message); + expect(data).to.not.have.a.property('result').that.is.an('object'); + expect(data).to.have.a.property('error').that.is.an('object'); + }); + await request + .get(api('chat.getMessage')) + .set(credentials) + .query({ msgId }) + .expect('Content-Type', 'application/json') + .expect(400) + .expect((res) => { + expect(res.body).to.have.property('success', false); + }); + }); }); describe('[@updateMessage]', () => { @@ -3108,4 +3149,143 @@ describe('Meteor.methods', function () { }); }); }); + + describe('[@saveSettings]', () => { + it('should return an error when trying to save a "NaN" value', () => { + request + .post(api('method.call/saveSettings')) + .set(credentials) + .send({ + message: JSON.stringify({ + msg: 'method', + id: '13', + method: 'saveSettings', + params: [[{ _id: 'Message_AllowEditing_BlockEditInMinutes', value: { $InfNaN: 0 } }]], + }), + }) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + const parsedBody = JSON.parse(res.body.message); + expect(parsedBody).to.have.property('error'); + expect(parsedBody.error).to.have.property('error', 'Invalid setting value NaN'); + }); + }); + + it('should return an error when trying to save a "Infinity" value', () => { + request + .post(api('method.call/saveSettings')) + .set(credentials) + .send({ + message: JSON.stringify({ + msg: 'method', + id: '13', + method: 'saveSettings', + params: [[{ _id: 'Message_AllowEditing_BlockEditInMinutes', value: { $InfNaN: 1 } }]], + }), + }) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + const parsedBody = JSON.parse(res.body.message); + expect(parsedBody).to.have.property('error'); + expect(parsedBody.error).to.have.property('error', 'Invalid setting value Infinity'); + }); + }); + + it('should return an error when trying to save a "-Infinity" value', () => { + request + .post(api('method.call/saveSettings')) + .set(credentials) + .send({ + message: JSON.stringify({ + msg: 'method', + id: '13', + method: 'saveSettings', + params: [[{ _id: 'Message_AllowEditing_BlockEditInMinutes', value: { $InfNaN: -1 } }]], + }), + }) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + const parsedBody = JSON.parse(res.body.message); + expect(parsedBody).to.have.property('error'); + expect(parsedBody.error).to.have.property('error', 'Invalid setting value -Infinity'); + }); + }); + }); + + describe('@insertOrUpdateUser', () => { + let testUser; + let testUserCredentials; + + before(async () => { + testUser = await createUser(); + testUserCredentials = await login(testUser.username, password); + }); + + after(() => Promise.all([deleteUser(testUser)])); + + it('should fail if user tries to verify their own email via insertOrUpdateUser', (done) => { + request + .post(methodCall('insertOrUpdateUser')) + .set(testUserCredentials) + .send({ + message: JSON.stringify({ + method: 'insertOrUpdateUser', + params: [ + { + _id: testUserCredentials['X-User-Id'], + email: 'manager@rocket.chat', + verified: true, + }, + ], + id: '52', + msg: 'method', + }), + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.a.property('success', true); + expect(res.body).to.have.a.property('message').that.is.a('string'); + const data = JSON.parse(res.body.message); + expect(data).to.have.a.property('msg', 'result'); + expect(data).to.have.a.property('id', '52'); + expect(data.error).to.have.property('error', 'error-action-not-allowed'); + }) + .end(done); + }); + + it('should pass if a user with the right permissions tries to verify the email of another user', (done) => { + request + .post(methodCall('insertOrUpdateUser')) + .set(credentials) + .send({ + message: JSON.stringify({ + method: 'insertOrUpdateUser', + params: [ + { + _id: testUserCredentials['X-User-Id'], + email: 'testuser@rocket.chat', + verified: true, + }, + ], + id: '52', + msg: 'method', + }), + }) + .expect('Content-Type', 'application/json') + .expect(200) + .expect((res) => { + expect(res.body).to.have.a.property('success', true); + expect(res.body).to.have.a.property('message').that.is.a('string'); + const data = JSON.parse(res.body.message); + expect(data).to.have.a.property('msg', 'result'); + expect(data).to.have.a.property('id', '52'); + expect(data).to.have.a.property('result', true); + }) + .end(done); + }); + }); }); diff --git a/apps/meteor/tests/end-to-end/api/methods/2fa-enable.ts b/apps/meteor/tests/end-to-end/api/methods/2fa-enable.ts new file mode 100644 index 000000000000..a2bd9dfeb3a7 --- /dev/null +++ b/apps/meteor/tests/end-to-end/api/methods/2fa-enable.ts @@ -0,0 +1,158 @@ +import type { IUser } from '@rocket.chat/core-typings'; +import { Random } from '@rocket.chat/random'; +import { expect } from 'chai'; +import { before, describe, it, after } from 'mocha'; +import speakeasy from 'speakeasy'; + +import { getCredentials, methodCall, request } from '../../../data/api-data'; +import { password } from '../../../data/user'; +import { createUser, deleteUser, login } from '../../../data/users.helper'; + +describe('2fa:enable', function () { + this.retries(0); + let totpSecret: string; + let user1: IUser; + let user2: IUser; + let user3: IUser; + let user1Credentials: { 'X-Auth-Token': string; 'X-User-Id': string }; + let user2Credentials: { 'X-Auth-Token': string; 'X-User-Id': string }; + let user3Credentials: { 'X-Auth-Token': string; 'X-User-Id': string }; + + before((done) => getCredentials(done)); + + before('create user', async () => { + [user1, user2, user3] = await Promise.all([ + createUser({ username: Random.id(), email: `${Random.id()}@example.com`, verified: true }), + createUser({ username: Random.id(), email: `${Random.id()}@example.com}`, verified: true }), + createUser({ username: Random.id(), email: `${Random.id()}@example.com}`, verified: false }), + ]); + [user1Credentials, user2Credentials, user3Credentials] = await Promise.all([ + login(user1.username, password), + login(user2.username, password), + login(user3.username, password), + ]); + }); + + after('remove user', async () => Promise.all([deleteUser(user1), deleteUser(user2), deleteUser(user3)])); + + it('should return error when user is not logged in', async () => { + await request + .post(methodCall('2fa:enable')) + .send({ + message: JSON.stringify({ + msg: 'method', + id: 'id1', + method: '2fa:enable', + params: [], + }), + }) + .expect(401) + .expect((res) => { + expect(res.body).to.have.property('status', 'error'); + }); + }); + + it('should return error when user is not verified', async () => { + await request + .post(methodCall('2fa:enable')) + .set(user3Credentials) + .send({ + message: JSON.stringify({ + msg: 'method', + id: 'id1', + method: '2fa:enable', + params: [], + }), + }) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('message'); + const result = JSON.parse(res.body.message); + expect(result).to.have.property('error'); + expect(result.error).to.not.have.property('errpr', 'error-invalid-user'); + }); + }); + + it('should return secret and qr code url when 2fa is disabled on user', async () => { + await request + .post(methodCall('2fa:enable')) + .set(user1Credentials) + .send({ + message: JSON.stringify({ + msg: 'method', + id: 'id1', + method: '2fa:enable', + params: [], + }), + }) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + const parsedBody = JSON.parse(res.body.message); + expect(parsedBody).to.have.property('result'); + expect(parsedBody.result).to.have.property('secret').of.a('string'); + expect(parsedBody.result) + .to.have.property('url') + .of.a('string') + .match(/^otpauth:\/\//); + }); + }); + + it('should enable 2fa on the user', async () => { + const enableResponse = await request + .post(methodCall('2fa:enable')) + .set(user2Credentials) + .send({ + message: JSON.stringify({ + msg: 'method', + id: 'id2', + method: '2fa:enable', + params: [], + }), + }) + .expect(200); + + const enableData = JSON.parse(enableResponse.body.message); + totpSecret = enableData.result.secret; + + await request + .post(methodCall('2fa:validateTempToken')) + .set(user2Credentials) + .send({ + message: JSON.stringify({ + msg: 'method', + id: 'id3', + method: '2fa:validateTempToken', + params: [speakeasy.totp({ secret: totpSecret, encoding: 'base32' })], + }), + }) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + const parsedBody = JSON.parse(res.body.message); + expect(parsedBody).to.have.property('result'); + expect(parsedBody.result).to.have.property('codes').of.a('array'); + }); + }); + + it('should return error when 2fa is already enabled on the user', async () => { + await request + .post(methodCall('2fa:enable')) + .set(user2Credentials) + .send({ + message: JSON.stringify({ + msg: 'method', + id: 'id4', + method: '2fa:enable', + params: [], + }), + }) + .expect(200) + .expect((res) => { + expect(res.body).to.have.property('success', true); + const parsedBody = JSON.parse(res.body.message); + expect(parsedBody).to.have.property('error'); + expect(parsedBody).to.not.have.property('result'); + }); + }); +}); diff --git a/apps/meteor/tests/unit/app/lib/server/lib/checkUrlForSsrf.tests.ts b/apps/meteor/tests/unit/app/lib/server/lib/checkUrlForSsrf.tests.ts new file mode 100644 index 000000000000..9cb7f1cd288c --- /dev/null +++ b/apps/meteor/tests/unit/app/lib/server/lib/checkUrlForSsrf.tests.ts @@ -0,0 +1,52 @@ +import { expect } from 'chai'; + +import { checkUrlForSsrf } from '../../../../../../app/lib/server/functions/checkUrlForSsrf'; + +describe('checkUrlForSsrf', () => { + it('should return false if the URL does not start with http:// or https://', async () => { + const result = await checkUrlForSsrf('ftp://example.com'); + expect(result).to.be.false; + }); + + it('should return false if the domain is not valid', async () => { + const result = await checkUrlForSsrf('https://www_google_com'); + expect(result).to.be.false; + }); + + it('should return false if the IP is not in a valid IPv4 format', async () => { + const result = await checkUrlForSsrf('https://127.1'); + expect(result).to.be.false; + }); + + it('should return false if the IP is in a restricted range', async () => { + const result = await checkUrlForSsrf('http://127.0.0.1'); + expect(result).to.be.false; + }); + + it('should return false if the domain is metadata.google.internal', async () => { + const result = await checkUrlForSsrf('http://metadata.google.internal'); + expect(result).to.be.false; + }); + + it('should return false if DNS resolves to an IP in the restricted range', async () => { + const result = await checkUrlForSsrf('http://169.254.169.254.nip.io'); + expect(result).to.be.false; + }); + + it('should return true if valid domain', async () => { + const result = await checkUrlForSsrf('https://www.google.com/'); + expect(result).to.be.true; + }); + + it('should return true if valid IP', async () => { + const result = await checkUrlForSsrf('http://216.58.214.174'); + expect(result).to.be.true; + }); + + it('should return true if valid URL', async () => { + const result = await checkUrlForSsrf( + 'https://upload.wikimedia.org/wikipedia/commons/thumb/1/15/Cat_August_2010-4.jpg/2560px-Cat_August_2010-4.jpg', + ); + expect(result).to.be.true; + }); +}); diff --git a/apps/meteor/tests/unit/app/lib/server/lib/validateNameChars.tests.ts b/apps/meteor/tests/unit/app/lib/server/lib/validateNameChars.tests.ts new file mode 100644 index 000000000000..a78a0cbc3322 --- /dev/null +++ b/apps/meteor/tests/unit/app/lib/server/lib/validateNameChars.tests.ts @@ -0,0 +1,56 @@ +import { expect } from 'chai'; + +import { validateNameChars } from '../../../../../../app/lib/server/functions/validateNameChars'; + +describe('validateNameChars', () => { + it('should return false for undefined input', () => { + expect(validateNameChars(undefined)).to.be.false; + }); + + it('should return false for non-string input', () => { + expect(validateNameChars(123 as any)).to.be.false; + expect(validateNameChars({} as any)).to.be.false; + expect(validateNameChars([] as any)).to.be.false; + }); + + it('should return false for names with invalid characters', () => { + expect(validateNameChars('name<')).to.be.false; + expect(validateNameChars('name>')).to.be.false; + expect(validateNameChars('name/')).to.be.false; + expect(validateNameChars('name\\')).to.be.false; + }); + + it('should return false for names with invalid characters after decoding', () => { + expect(validateNameChars('name%3E')).to.be.false; + expect(validateNameChars('name%5C')).to.be.false; + expect(validateNameChars('name%3C')).to.be.false; + }); + + it('should return false for malicious HTML payloads', () => { + expect(validateNameChars('')).to.be.false; + expect(validateNameChars('%3Cscript%3Ealert%28%27XSS%27%29%3C%2Fscript%3E')).to.be.false; + expect( + validateNameChars( + '
', + ), + ).to.be.false; + expect( + validateNameChars( + '%3Cform%20action%3D%22http%3A%2F%2Fmalicious.site%22%20method%3D%22post%22%3E%3Cinput%20type%3D%22text%22%20name%3D%22username%22%20value%3D%22Enter%20username%22%3E%3Cinput%20type%3D%22password%22%20name%3D%22password%22%20value%3D%22Enter%20password%22%3E%3Cinput%20type%3D%22submit%22%20value%3D%22Submit%22%3E%3C%2Fform%3E', + ), + ).to.be.false; + }); + + it('should return false if decodeURI throws an error', () => { + expect(validateNameChars('%')).to.be.false; + expect(validateNameChars('%E0%A4%A')).to.be.false; + }); + + it('should return true for valid names', () => { + expect(validateNameChars('name')).to.be.true; + expect(validateNameChars('valid_name')).to.be.true; + expect(validateNameChars('valid-name')).to.be.true; + expect(validateNameChars('valid.name')).to.be.true; + expect(validateNameChars('valid name')).to.be.true; + }); +}); diff --git a/packages/i18n/src/locales/en.i18n.json b/packages/i18n/src/locales/en.i18n.json index 24fb91023e1a..617791c5de0c 100644 --- a/packages/i18n/src/locales/en.i18n.json +++ b/packages/i18n/src/locales/en.i18n.json @@ -6043,6 +6043,7 @@ "registration.component.form.emailOrUsername": "Email or username", "registration.component.form.username": "Username", "registration.component.form.name": "Name", + "registration.component.form.nameContainsInvalidChars": "Name contains invalid characters", "registration.component.form.nameOptional": "Name optional", "registration.component.form.createAnAccount": "Create an account", "registration.component.form.userAlreadyExist": "Username already exists. Please try another username.", diff --git a/packages/i18n/src/locales/pt-BR.i18n.json b/packages/i18n/src/locales/pt-BR.i18n.json index 0cc43f6fb508..abddd5601229 100644 --- a/packages/i18n/src/locales/pt-BR.i18n.json +++ b/packages/i18n/src/locales/pt-BR.i18n.json @@ -4916,6 +4916,7 @@ "registration.component.resetPassword": "Redefinir senha", "registration.component.form.username": "Nome de usuário", "registration.component.form.name": "Nome", + "registration.component.form.nameContainsInvalidChars": "O nome contém caracteres inválidos", "registration.component.form.userAlreadyExist": "O nome de usuário já existe. Tente outro nome de usuário.", "registration.component.form.emailAlreadyExists": "E-mail já existe", "registration.component.form.usernameAlreadyExists": "O nome de usuário já existe. Tente outro nome de usuário.", diff --git a/packages/i18n/src/locales/pt.i18n.json b/packages/i18n/src/locales/pt.i18n.json index c3bf9d739b21..f760968e335e 100644 --- a/packages/i18n/src/locales/pt.i18n.json +++ b/packages/i18n/src/locales/pt.i18n.json @@ -3174,6 +3174,7 @@ "registration.component.form.emailOrUsername": "Email ou nome de utilizador", "registration.component.form.username": "Nome de utilizador", "registration.component.form.name": "Nome", + "registration.component.form.nameContainsInvalidChars": "O nome contém caracteres inválidos", "registration.component.form.userAlreadyExist": "O nome de utilizador já existe. Por favor, tente outro nome de utilizador.", "registration.component.form.emailAlreadyExists": "Email já registado", "registration.component.form.usernameAlreadyExists": "O nome de utilizador já existe. Por favor, tente outro nome de utilizador.", diff --git a/packages/web-ui-registration/src/RegisterForm.tsx b/packages/web-ui-registration/src/RegisterForm.tsx index 0eda77879be7..57cf9378ab72 100644 --- a/packages/web-ui-registration/src/RegisterForm.tsx +++ b/packages/web-ui-registration/src/RegisterForm.tsx @@ -94,14 +94,15 @@ export const RegisterForm = ({ setLoginRoute }: { setLoginRoute: DispatchLoginRo if (error.errorType === 'error-user-already-exists') { setError('username', { type: 'user-already-exists', message: t('registration.component.form.usernameAlreadyExists') }); } - if (/Email already exists/.test(error.error)) { setError('email', { type: 'email-already-exists', message: t('registration.component.form.emailAlreadyExists') }); } - if (/Username is already in use/.test(error.error)) { setError('username', { type: 'username-already-exists', message: t('registration.component.form.userAlreadyExist') }); } + if (/Name contains invalid characters/.test(error.error)) { + setError('name', { type: 'name-contains-invalid-chars', message: t('registration.component.form.nameContainsInvalidChars') }); + } if (/error-too-many-requests/.test(error.error)) { dispatchToastMessage({ type: 'error', message: error.error }); }