diff --git a/apps/meteor/app/authentication/server/lib/restrictLoginAttempts.ts b/apps/meteor/app/authentication/server/lib/restrictLoginAttempts.ts index 300a4fa973114..1bf9c1dd6cbc6 100644 --- a/apps/meteor/app/authentication/server/lib/restrictLoginAttempts.ts +++ b/apps/meteor/app/authentication/server/lib/restrictLoginAttempts.ts @@ -2,6 +2,7 @@ import type { IServerEvent } from '@rocket.chat/core-typings'; import { ServerEventType } from '@rocket.chat/core-typings'; import { Logger } from '@rocket.chat/logger'; import { Rooms, ServerEvents, Users } from '@rocket.chat/models'; +import { isIpInCidrRange } from '@rocket.chat/server-fetch'; import { addMinutesToADate } from '../../../../lib/utils/addMinutesToADate'; import { getClientAddress } from '../../../../server/lib/getClientAddress'; @@ -52,7 +53,15 @@ export const isValidLoginAttemptByIp = async (ip: string): Promise => { if ( !settings.get('Block_Multiple_Failed_Logins_Enabled') || !settings.get('Block_Multiple_Failed_Logins_By_Ip') || - whitelist.includes(ip) + whitelist.some((entry) => { + const trimmed = entry.trim(); + try { + return trimmed === ip || isIpInCidrRange(ip, trimmed); + } catch (error) { + logger.error({ msg: 'Invalid CIDR entry in whitelist', entry: trimmed, error }); + return false; + } + }) ) { return true; } @@ -100,7 +109,9 @@ export const isValidAttemptByUser = async (login: ILoginAttempt): Promise => { const user: IServerEvent['u'] = { _id: login.user?._id, - username: login.user?.username || login.methodArguments[0].user?.username, + username: login.user?.username || login.methodArguments[0].user?.username || login.methodArguments[0].user?.email, }; await ServerEvents.insertOne({ @@ -161,7 +172,7 @@ export const saveFailedLoginAttempts = async (login: ILoginAttempt): Promise => { const user: IServerEvent['u'] = { _id: login.user?._id, - username: login.user?.username || login.methodArguments[0].user?.username, + username: login.user?.username || login.methodArguments[0].user?.username || login.methodArguments[0].user?.email, }; await ServerEvents.insertOne({ diff --git a/apps/meteor/lib/utils/isRelativeURL.ts b/apps/meteor/lib/utils/isRelativeURL.ts index aa682693df860..c289102206a29 100644 --- a/apps/meteor/lib/utils/isRelativeURL.ts +++ b/apps/meteor/lib/utils/isRelativeURL.ts @@ -1 +1,17 @@ -export const isRelativeURL = (str: string): boolean => /^[^\/]+\/[^\/].*$|^\/[^\/].*$/gim.test(str); +export const isRelativeURL = (str: string): boolean => { + // Scheme-based URLs are absolute (e.g. https:, data:, javascript:). + if (/^[a-z][a-z\d+\-.]*:/i.test(str)) { + return false; + } + + // URLs starting with // are protocol-relative, not path-relative. + if (str.startsWith('//')) { + return false; + } + + if (str === '/' || str === '') { + return false; + } + + return true; +}; diff --git a/apps/meteor/tests/unit/lib/utils/isRelativeURL.spec.ts b/apps/meteor/tests/unit/lib/utils/isRelativeURL.spec.ts index 86c054f0fcc11..d66dbb4a73a18 100644 --- a/apps/meteor/tests/unit/lib/utils/isRelativeURL.spec.ts +++ b/apps/meteor/tests/unit/lib/utils/isRelativeURL.spec.ts @@ -5,12 +5,16 @@ import { isRelativeURL } from '../../../../lib/utils/isRelativeURL'; describe('isRelativeURL', () => { const testCases = [ ['/', false], - ['test', false], // TODO: should be true? + ['', false], + ['test', true], ['test/test', true], - ['.', false], // TODO: should be true? + ['.', true], ['./test', true], + ['../test', true], + ['/test', true], + ['//rocket.chat/path', false], ['https://rocket.chat', false], - ['data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==', true], // TODO: should be false? + ['data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==', false], ] as const; testCases.forEach(([parameter, expectedResult]) => { diff --git a/packages/server-fetch/src/helpers.ts b/packages/server-fetch/src/helpers.ts index 69e10710f39aa..841f45c1efc14 100644 --- a/packages/server-fetch/src/helpers.ts +++ b/packages/server-fetch/src/helpers.ts @@ -50,7 +50,13 @@ export const isIpInCidrRange = (ip: string, cidr: string): boolean => { groups.push(part); } } - return groups.reduce((value, hexGroup) => (value << 16n) + BigInt(parseInt(hexGroup, 16)), 0n); + return groups.reduce((value, hexGroup) => { + const parsed = parseInt(hexGroup, 16); + if (isNaN(parsed)) { + return value << 16n; + } + return (value << 16n) + BigInt(parsed); + }, 0n); }; const mask = prefix === 0 ? 0n : ((1n << BigInt(prefix)) - 1n) << (128n - BigInt(prefix)); return (toBigInt(ip) & mask) === (toBigInt(network) & mask); diff --git a/packages/server-fetch/src/index.ts b/packages/server-fetch/src/index.ts index 2355d2a1b4754..4a693c6576fea 100644 --- a/packages/server-fetch/src/index.ts +++ b/packages/server-fetch/src/index.ts @@ -194,5 +194,6 @@ export async function serverFetch(input: string, options?: ExtendedFetchOptions, } export { Response }; +export { isIpInCidrRange } from './helpers'; export type { ExtendedFetchOptions }; export { parseSsrfAllowlist }; diff --git a/packages/server-fetch/tests/checkForSsrf.spec.ts b/packages/server-fetch/tests/checkForSsrf.spec.ts index aceb4f69889c5..a08f442399214 100644 --- a/packages/server-fetch/tests/checkForSsrf.spec.ts +++ b/packages/server-fetch/tests/checkForSsrf.spec.ts @@ -297,9 +297,10 @@ describe('checkForSsrfHelpers', () => { expect(isIpInCidrRange('::1', '127.0.0.0/8')).toBe(false); }); }); - it('returns false for invalid IP', () => { + it('returns false for malformed IP or CIDR', () => { expect(isIpInCidrRange('not-an-ip', '192.168.0.0/16')).toBe(false); expect(isIpInCidrRange('256.1.1.1', '192.168.0.0/16')).toBe(false); + expect(isIpInCidrRange('::1', '::g/128')).toBe(false); // Malformed hex group }); });