Skip to content

Commit

Permalink
fix: imported fixes (#32894) (#32919)
Browse files Browse the repository at this point in the history
Co-authored-by: Diego Sampaio <[email protected]>
Co-authored-by: Julio A. <[email protected]>
  • Loading branch information
3 people committed Jul 29, 2024
1 parent 4f08440 commit ce21727
Show file tree
Hide file tree
Showing 22 changed files with 708 additions and 8 deletions.
5 changes: 5 additions & 0 deletions .changeset/hungry-jars-lay.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)
4 changes: 4 additions & 0 deletions apps/meteor/app/2fa/server/methods/enable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ Meteor.methods<ServerMethods>({
});
}

if (user.services?.totp?.enabled) {
throw new Meteor.Error('error-2fa-already-enabled');
}

const secret = TOTP.generateSecret();

await Users.disable2FAAndSetTempSecretByUserId(userId, secret.base32);
Expand Down
5 changes: 5 additions & 0 deletions apps/meteor/app/api/server/v1/chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<IMessage, 'rid'>, this.bodyParams.previewUrls);
const [message] = await normalizeMessagesForUser([sent], this.userId);

Expand Down
21 changes: 19 additions & 2 deletions apps/meteor/app/api/server/v1/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down
100 changes: 100 additions & 0 deletions apps/meteor/app/lib/server/functions/checkUrlForSsrf.ts
Original file line number Diff line number Diff line change
@@ -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<string> => {
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}(?<!-)\.?([A-Za-z]{2,63}\.?)*[A-Za-z]{2,63}$/;
if (!domainPattern.test(domain)) {
return false;
}
return true;
};

export const checkUrlForSsrf = async (url: string): Promise<boolean> => {
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;
};
7 changes: 7 additions & 0 deletions apps/meteor/app/lib/server/functions/saveUser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
12 changes: 11 additions & 1 deletion apps/meteor/app/lib/server/functions/setUserAvatar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)}`, {
Expand Down
21 changes: 21 additions & 0 deletions apps/meteor/app/lib/server/functions/validateNameChars.ts
Original file line number Diff line number Diff line change
@@ -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;
};
7 changes: 7 additions & 0 deletions apps/meteor/app/lib/server/methods/sendMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -78,6 +79,8 @@ export async function executeSendMessage(uid: IUser['_id'], message: AtLeast<IMe
throw new Error("The 'rid' property on the message object is missing.");
}

check(rid, String);

try {
const room = await canSendMessageAsync(rid, { uid, username: user.username, type: user.type });

Expand Down Expand Up @@ -117,6 +120,10 @@ Meteor.methods<ServerMethods>({
});
}

if (MessageTypes.isSystemMessage(message)) {
throw new Error("Cannot send system messages using 'sendMessage'");
}

try {
return await executeSendMessage(uid, message, previewUrls);
} catch (error: any) {
Expand Down
8 changes: 7 additions & 1 deletion apps/meteor/app/livechat/imports/server/rest/sms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,20 @@ 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';

const logger = new Logger('SMS');

const getUploadFile = async (details: Omit<IUpload, '_id'>, 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());

Expand Down
10 changes: 9 additions & 1 deletion apps/meteor/server/routes/avatar/room.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 } });
Expand Down Expand Up @@ -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);
};
8 changes: 7 additions & 1 deletion apps/meteor/server/routes/avatar/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(/\?.*$/, ''));
Expand All @@ -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);

Expand Down
Loading

0 comments on commit ce21727

Please sign in to comment.