From 855a9dec685ae545dd58ecffeb961813e9b3b1d3 Mon Sep 17 00:00:00 2001 From: Soumajit Ghosh Date: Tue, 14 Apr 2026 08:17:04 +0530 Subject: [PATCH 1/7] fix: prevent unbounded memory consumption in avatar URL uploads Remote avatar URLs fetched via `setUserAvatar` loaded entire response into memory with no size limit, enabling OOM via large files or concurrent requests. - Stream response with byte-limit enforcement (FileUpload_MaxFileSize) - Early-reject on oversized Content-Length header - Abort mid-stream when actual bytes exceed limit regardless of header - Add 20s body-read timeout to prevent slow-body resource hold - Destroy response body on non-200 / non-image paths to free sockets - Add unit tests for: oversized Content-Length, missing header, lied header, non-image MIME, valid small image, body-read timeout --- .../app/lib/server/functions/setUserAvatar.ts | 116 +++++++++- .../server/functions/setUserAvatar.spec.ts | 203 ++++++++++++++++++ 2 files changed, 314 insertions(+), 5 deletions(-) create mode 100644 apps/meteor/tests/unit/app/lib/server/functions/setUserAvatar.spec.ts diff --git a/apps/meteor/app/lib/server/functions/setUserAvatar.ts b/apps/meteor/app/lib/server/functions/setUserAvatar.ts index d65e73a887d63..1470b48d31a61 100644 --- a/apps/meteor/app/lib/server/functions/setUserAvatar.ts +++ b/apps/meteor/app/lib/server/functions/setUserAvatar.ts @@ -1,3 +1,5 @@ +import type { Readable } from 'stream'; + import { api } from '@rocket.chat/core-services'; import type { IUser } from '@rocket.chat/core-typings'; import type { Updater } from '@rocket.chat/models'; @@ -14,6 +16,79 @@ import { RocketChatFile } from '../../../file/server'; import { FileUpload } from '../../../file-upload/server'; import { settings } from '../../../settings/server'; +const DEFAULT_AVATAR_DOWNLOAD_TIMEOUT_MS = 20_000; +const DEFAULT_MAX_FILE_SIZE = 104_857_600; +const AVATAR_RESPONSE_TOO_LARGE = 'error-avatar-response-too-large'; +const AVATAR_RESPONSE_TIMEOUT = 'error-avatar-response-timeout'; + +const discardResponseBody = (response: Pick): void => { + const body = response.body as Partial | null; + + body?.resume?.(); + body?.destroy?.(); +}; + +const bufferResponseWithLimit = async (response: Response, sizeLimit: number, timeoutMs: number): Promise => { + const contentLength = Number.parseInt(response.headers.get('content-length') || '', 10); + + if (Number.isFinite(contentLength) && contentLength > sizeLimit) { + discardResponseBody(response); + throw new Error(AVATAR_RESPONSE_TOO_LARGE); + } + + if (!response.body) { + return Buffer.alloc(0); + } + + return new Promise((resolve, reject) => { + const body = response.body as Readable; + const chunks: Buffer[] = []; + let totalBytes = 0; + + const cleanup = (): void => { + clearTimeout(timeoutId); + body.removeListener('data', onData); + body.removeListener('end', onEnd); + body.removeListener('error', onError); + }; + + const rejectWithError = (error: Error): void => { + cleanup(); + body.destroy(); + reject(error); + }; + + const onData = (chunk: Buffer | string): void => { + const bufferChunk = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); + totalBytes += bufferChunk.length; + + if (totalBytes > sizeLimit) { + rejectWithError(new Error(AVATAR_RESPONSE_TOO_LARGE)); + return; + } + + chunks.push(bufferChunk); + }; + + const onEnd = (): void => { + cleanup(); + resolve(Buffer.concat(chunks)); + }; + + const onError = (error: Error): void => { + cleanup(); + reject(error); + }; + + const timeoutId = setTimeout(() => rejectWithError(new Error(AVATAR_RESPONSE_TIMEOUT)), timeoutMs); + + body.on('data', onData); + body.once('end', onEnd); + body.once('error', onError); + body.resume(); + }); +}; + export const setAvatarFromServiceWithValidation = async ( userId: string, dataURI: string, @@ -101,11 +176,13 @@ export async function setUserAvatar( const { buffer, type } = await (async (): Promise<{ buffer: Buffer; type: string }> => { if (service === 'url' && typeof dataURI === 'string') { let response: Response; + const maxFileSize = Number(settings.get('FileUpload_MaxFileSize')) || DEFAULT_MAX_FILE_SIZE; try { response = await fetch(dataURI, { ignoreSsrfValidation: false, allowList: settings.get('SSRF_Allowlist'), + timeout: DEFAULT_AVATAR_DOWNLOAD_TIMEOUT_MS, }); } catch (e) { SystemLogger.info({ @@ -120,6 +197,8 @@ export async function setUserAvatar( } if (response.status !== 200) { + discardResponseBody(response); + if (response.status !== 404) { SystemLogger.info({ msg: 'Error while handling the setting of the avatar from a url', @@ -146,6 +225,8 @@ export async function setUserAvatar( } if (!/image\/.+/.test(response.headers.get('content-type') || '')) { + discardResponseBody(response); + SystemLogger.info({ msg: 'Not a valid content-type from the provided avatar url', contentType: response.headers.get('content-type'), @@ -157,10 +238,33 @@ export async function setUserAvatar( }); } - return { - buffer: Buffer.from(await response.arrayBuffer()), - type: response.headers.get('content-type') || '', - }; + try { + return { + buffer: await bufferResponseWithLimit(response, maxFileSize, DEFAULT_AVATAR_DOWNLOAD_TIMEOUT_MS), + type: response.headers.get('content-type') || '', + }; + } catch (error) { + if (error instanceof Error && error.message === AVATAR_RESPONSE_TOO_LARGE) { + throw new Meteor.Error('error-file-too-large', 'Avatar file exceeds allowed size limit', { + function: 'setUserAvatar', + url: dataURI, + sizeLimit: maxFileSize, + }); + } + + SystemLogger.info({ + msg: 'Error while downloading avatar from provided url', + url: encodeURI(dataURI), + username: user.username, + err: error, + }); + + throw new Meteor.Error( + 'error-avatar-url-handling', + `Error while handling avatar setting from a URL (${encodeURI(dataURI)}) for ${user.username}`, + { function: 'RocketChat.setUserAvatar', url: dataURI, username: user.username }, + ); + } } if (service === 'rest') { @@ -185,7 +289,9 @@ export async function setUserAvatar( })(); const fileStore = FileUpload.getStore('Avatars'); - user.username && (await fileStore.deleteByName(user.username, { session })); + if (user.username) { + await fileStore.deleteByName(user.username, { session }); + } const file = { userId: user._id, diff --git a/apps/meteor/tests/unit/app/lib/server/functions/setUserAvatar.spec.ts b/apps/meteor/tests/unit/app/lib/server/functions/setUserAvatar.spec.ts new file mode 100644 index 0000000000000..299fce4f7da07 --- /dev/null +++ b/apps/meteor/tests/unit/app/lib/server/functions/setUserAvatar.spec.ts @@ -0,0 +1,203 @@ +import { Readable } from 'stream'; + +import { expect } from 'chai'; +import proxyquire from 'proxyquire'; +import sinon from 'sinon'; + +class MeteorError extends Error { + constructor( + public error: string, + public reason?: string, + public details?: Record, + ) { + super(error); + } +} + +const createResponse = ({ + status = 200, + headers = {}, + body, +}: { + status?: number; + headers?: Record; + body?: Readable | null; +}) => ({ + status, + headers: { + get: (key: string) => headers[key.toLowerCase()] || null, + }, + body: body ?? null, +}); + +describe('setUserAvatar', () => { + const user = { _id: 'user-id', username: 'tester' }; + const fileStore = { + deleteByName: sinon.stub().resolves(), + insert: sinon.stub().resolves({ etag: 'etag' }), + }; + + const stubs = { + Users: { + setAvatarData: sinon.stub().resolves(), + findOneById: sinon.stub(), + }, + fetch: sinon.stub(), + settings: { + get: sinon.stub(), + }, + SystemLogger: { + info: sinon.stub(), + }, + FileUpload: { + getStore: sinon.stub().returns(fileStore), + }, + RocketChatFile: { + dataURIParse: sinon.stub(), + }, + api: { + broadcast: sinon.stub(), + }, + hasPermissionAsync: sinon.stub(), + }; + + const { setUserAvatar } = proxyquire.noCallThru().load('../../../../../../app/lib/server/functions/setUserAvatar', { + '@rocket.chat/core-services': { api: stubs.api }, + '@rocket.chat/models': { Users: stubs.Users }, + '@rocket.chat/server-fetch': { serverFetch: stubs.fetch }, + 'meteor/meteor': { Meteor: { Error: MeteorError } }, + '../../../../server/database/utils': { onceTransactionCommitedSuccessfully: async (cb: any, _sess: any) => cb() }, + '../../../../server/lib/logger/system': { SystemLogger: stubs.SystemLogger }, + '../../../authorization/server/functions/hasPermission': { hasPermissionAsync: stubs.hasPermissionAsync }, + '../../../file/server': { RocketChatFile: stubs.RocketChatFile }, + '../../../file-upload/server': { FileUpload: stubs.FileUpload }, + '../../../settings/server': { settings: stubs.settings }, + }); + + beforeEach(() => { + stubs.settings.get.callsFake((key: string) => { + if (key === 'SSRF_Allowlist') { + return '*'; + } + + if (key === 'FileUpload_MaxFileSize') { + return 4; + } + + return undefined; + }); + }); + + afterEach(() => { + sinon.restore(); + fileStore.deleteByName.resetHistory(); + fileStore.insert.resetHistory(); + stubs.Users.setAvatarData.resetHistory(); + stubs.fetch.resetHistory(); + stubs.settings.get.resetHistory(); + stubs.SystemLogger.info.resetHistory(); + stubs.api.broadcast.resetHistory(); + }); + + it('rejects avatar url when content-length exceeds max size', async () => { + stubs.fetch.resolves( + createResponse({ + headers: { + 'content-type': 'image/png', + 'content-length': '10', + }, + body: Readable.from([Buffer.from('1234')]), + }), + ); + + await expect(setUserAvatar(user, 'https://example.com/avatar.png', '', 'url')).to.be.rejectedWith('error-file-too-large'); + expect(fileStore.insert.called).to.be.false; + }); + + it('rejects avatar url when streamed body exceeds max size without content-length', async () => { + stubs.fetch.resolves( + createResponse({ + headers: { + 'content-type': 'image/png', + }, + body: Readable.from([Buffer.from('12'), Buffer.from('345')]), + }), + ); + + await expect(setUserAvatar(user, 'https://example.com/avatar.png', '', 'url')).to.be.rejectedWith('error-file-too-large'); + expect(fileStore.insert.called).to.be.false; + }); + + it('rejects avatar url when body exceeds lied content-length', async () => { + stubs.fetch.resolves( + createResponse({ + headers: { + 'content-type': 'image/png', + 'content-length': '2', + }, + body: Readable.from([Buffer.from('12345')]), + }), + ); + + await expect(setUserAvatar(user, 'https://example.com/avatar.png', '', 'url')).to.be.rejectedWith('error-file-too-large'); + expect(fileStore.insert.called).to.be.false; + }); + + it('rejects non-image avatar url content type', async () => { + stubs.fetch.resolves( + createResponse({ + headers: { + 'content-type': 'text/plain', + }, + body: Readable.from([Buffer.from('ok')]), + }), + ); + + await expect(setUserAvatar(user, 'https://example.com/avatar.txt', '', 'url')).to.be.rejectedWith('error-avatar-invalid-url'); + expect(fileStore.insert.called).to.be.false; + }); + + it('stores avatar when streamed image stays within limit', async () => { + stubs.fetch.resolves( + createResponse({ + headers: { + 'content-type': 'image/png', + 'content-length': '4', + }, + body: Readable.from([Buffer.from('12'), Buffer.from('34')]), + }), + ); + + await setUserAvatar(user, 'https://example.com/avatar.png', '', 'url'); + + expect(fileStore.insert.calledOnce).to.be.true; + expect(fileStore.insert.firstCall.args[0]).to.deep.include({ + userId: user._id, + type: 'image/png', + size: 4, + }); + expect(fileStore.insert.firstCall.args[1].equals(Buffer.from('1234'))).to.be.true; + }); + + it('rejects avatar url when body read times out', async () => { + const clock = sinon.useFakeTimers(); + const hangingStream = new Readable({ read: sinon.stub() }); + + stubs.fetch.resolves( + createResponse({ + headers: { + 'content-type': 'image/png', + }, + body: hangingStream, + }), + ); + + const avatarPromise = setUserAvatar(user, 'https://example.com/avatar.png', '', 'url'); + const assertion = expect(avatarPromise).to.be.rejectedWith('error-avatar-url-handling'); + await clock.tickAsync(20_001); + + await assertion; + expect(fileStore.insert.called).to.be.false; + clock.restore(); + }); +}); From edb956ed657a93a1e9615f371003b47f05dec51e Mon Sep 17 00:00:00 2001 From: Soumajit Ghosh Date: Tue, 14 Apr 2026 08:45:55 +0530 Subject: [PATCH 2/7] fix: ensure default max file size is used when configuration is negative and handle avatar download timeout error --- .../app/lib/server/functions/setUserAvatar.ts | 11 +++++- .../server/functions/setUserAvatar.spec.ts | 35 ++++++++++++++++++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/lib/server/functions/setUserAvatar.ts b/apps/meteor/app/lib/server/functions/setUserAvatar.ts index 1470b48d31a61..685d01571ebb4 100644 --- a/apps/meteor/app/lib/server/functions/setUserAvatar.ts +++ b/apps/meteor/app/lib/server/functions/setUserAvatar.ts @@ -176,7 +176,8 @@ export async function setUserAvatar( const { buffer, type } = await (async (): Promise<{ buffer: Buffer; type: string }> => { if (service === 'url' && typeof dataURI === 'string') { let response: Response; - const maxFileSize = Number(settings.get('FileUpload_MaxFileSize')) || DEFAULT_MAX_FILE_SIZE; + const maxFileSizeSetting = Number(settings.get('FileUpload_MaxFileSize')); + const maxFileSize = Number.isFinite(maxFileSizeSetting) && maxFileSizeSetting > 0 ? maxFileSizeSetting : DEFAULT_MAX_FILE_SIZE; try { response = await fetch(dataURI, { @@ -252,6 +253,14 @@ export async function setUserAvatar( }); } + if (error instanceof Error && error.message === AVATAR_RESPONSE_TIMEOUT) { + throw new Meteor.Error('error-avatar-download-timeout', 'Avatar download timed out', { + function: 'setUserAvatar', + url: dataURI, + timeoutMs: DEFAULT_AVATAR_DOWNLOAD_TIMEOUT_MS, + }); + } + SystemLogger.info({ msg: 'Error while downloading avatar from provided url', url: encodeURI(dataURI), diff --git a/apps/meteor/tests/unit/app/lib/server/functions/setUserAvatar.spec.ts b/apps/meteor/tests/unit/app/lib/server/functions/setUserAvatar.spec.ts index 299fce4f7da07..c3cbd397749dd 100644 --- a/apps/meteor/tests/unit/app/lib/server/functions/setUserAvatar.spec.ts +++ b/apps/meteor/tests/unit/app/lib/server/functions/setUserAvatar.spec.ts @@ -179,6 +179,39 @@ describe('setUserAvatar', () => { expect(fileStore.insert.firstCall.args[1].equals(Buffer.from('1234'))).to.be.true; }); + it('falls back to default max file size when setting is negative', async () => { + stubs.settings.get.callsFake((key: string) => { + if (key === 'SSRF_Allowlist') { + return '*'; + } + + if (key === 'FileUpload_MaxFileSize') { + return -1; + } + + return undefined; + }); + + stubs.fetch.resolves( + createResponse({ + headers: { + 'content-type': 'image/png', + 'content-length': '10', + }, + body: Readable.from([Buffer.from('1234567890')]), + }), + ); + + await setUserAvatar(user, 'https://example.com/avatar.png', '', 'url'); + + expect(fileStore.insert.calledOnce).to.be.true; + expect(fileStore.insert.firstCall.args[0]).to.deep.include({ + userId: user._id, + type: 'image/png', + size: 10, + }); + }); + it('rejects avatar url when body read times out', async () => { const clock = sinon.useFakeTimers(); const hangingStream = new Readable({ read: sinon.stub() }); @@ -193,7 +226,7 @@ describe('setUserAvatar', () => { ); const avatarPromise = setUserAvatar(user, 'https://example.com/avatar.png', '', 'url'); - const assertion = expect(avatarPromise).to.be.rejectedWith('error-avatar-url-handling'); + const assertion = expect(avatarPromise).to.be.rejectedWith('error-avatar-download-timeout'); await clock.tickAsync(20_001); await assertion; From 942b5c06d94b563d44ff0cd54f77664291055b6b Mon Sep 17 00:00:00 2001 From: Soumajit Ghosh Date: Tue, 14 Apr 2026 09:08:44 +0530 Subject: [PATCH 3/7] fix: enhance avatar URL handling and add error mapping for fetch timeouts --- .../app/lib/server/functions/setUserAvatar.ts | 234 +++++++++++------- .../server/functions/setUserAvatar.spec.ts | 23 ++ 2 files changed, 162 insertions(+), 95 deletions(-) diff --git a/apps/meteor/app/lib/server/functions/setUserAvatar.ts b/apps/meteor/app/lib/server/functions/setUserAvatar.ts index 685d01571ebb4..9af3265eb1c48 100644 --- a/apps/meteor/app/lib/server/functions/setUserAvatar.ts +++ b/apps/meteor/app/lib/server/functions/setUserAvatar.ts @@ -21,6 +21,53 @@ const DEFAULT_MAX_FILE_SIZE = 104_857_600; const AVATAR_RESPONSE_TOO_LARGE = 'error-avatar-response-too-large'; const AVATAR_RESPONSE_TIMEOUT = 'error-avatar-response-timeout'; +const getMediaType = (contentTypeHeader: string | null): string => { + const mediaType = contentTypeHeader?.split(';', 1)[0].trim().toLowerCase(); + + return mediaType || ''; +}; + +const isImageContentType = (contentTypeHeader: string | null): boolean => /^image\/[^;\s]+$/.test(getMediaType(contentTypeHeader)); + +const isRequestTimeoutError = (error: unknown): boolean => { + if (!error || typeof error !== 'object') { + return false; + } + + const { name, code, type } = error as { name?: string; code?: string; type?: string }; + + return name === 'AbortError' || code === 'ETIMEDOUT' || type === 'request-timeout'; +}; + +const getMaxFileSize = (): number => { + const maxFileSizeSetting = Number(settings.get('FileUpload_MaxFileSize')); + + return Number.isFinite(maxFileSizeSetting) && maxFileSizeSetting > 0 ? maxFileSizeSetting : DEFAULT_MAX_FILE_SIZE; +}; + +const throwInvalidAvatarUrl = (dataURI: string): never => { + throw new Meteor.Error('error-avatar-invalid-url', `Invalid avatar URL: ${encodeURI(dataURI)}`, { + function: 'setUserAvatar', + url: dataURI, + }); +}; + +const throwAvatarDownloadTimeout = (dataURI: string): never => { + throw new Meteor.Error('error-avatar-download-timeout', 'Avatar download timed out', { + function: 'setUserAvatar', + url: dataURI, + timeoutMs: DEFAULT_AVATAR_DOWNLOAD_TIMEOUT_MS, + }); +}; + +const throwAvatarUrlHandling = (dataURI: string, username: string): never => { + throw new Meteor.Error( + 'error-avatar-url-handling', + `Error while handling avatar setting from a URL (${encodeURI(dataURI)}) for ${username}`, + { function: 'RocketChat.setUserAvatar', url: dataURI, username }, + ); +}; + const discardResponseBody = (response: Pick): void => { const body = response.body as Partial | null; @@ -89,6 +136,95 @@ const bufferResponseWithLimit = async (response: Response, sizeLimit: number, ti }); }; +const getAvatarFromUrl = async ( + user: Pick & { username: string }, + dataURI: string, +): Promise<{ buffer: Buffer; type: string }> => { + const maxFileSize = getMaxFileSize(); + const response = await (async (): Promise => { + try { + return await fetch(dataURI, { + ignoreSsrfValidation: false, + allowList: settings.get('SSRF_Allowlist'), + timeout: DEFAULT_AVATAR_DOWNLOAD_TIMEOUT_MS, + }); + } catch (error) { + if (isRequestTimeoutError(error)) { + throwAvatarDownloadTimeout(dataURI); + } + + SystemLogger.info({ + msg: 'Not a valid response from the avatar url', + url: encodeURI(dataURI), + err: error, + }); + throwInvalidAvatarUrl(dataURI); + throw error; + } + })(); + + if (response.status !== 200) { + discardResponseBody(response); + + if (response.status !== 404) { + SystemLogger.info({ + msg: 'Error while handling the setting of the avatar from a url', + url: encodeURI(dataURI), + username: user.username, + status: response.status, + }); + throwAvatarUrlHandling(dataURI, user.username); + } + + SystemLogger.info({ + msg: 'Not a valid response from the avatar url', + status: response.status, + url: dataURI, + }); + throwInvalidAvatarUrl(dataURI); + } + + const type = response.headers.get('content-type') || ''; + if (!isImageContentType(type)) { + discardResponseBody(response); + + SystemLogger.info({ + msg: 'Not a valid content-type from the provided avatar url', + contentType: type, + url: dataURI, + }); + throwInvalidAvatarUrl(dataURI); + } + + try { + return { + buffer: await bufferResponseWithLimit(response, maxFileSize, DEFAULT_AVATAR_DOWNLOAD_TIMEOUT_MS), + type, + }; + } catch (error) { + if (error instanceof Error && error.message === AVATAR_RESPONSE_TOO_LARGE) { + throw new Meteor.Error('error-file-too-large', 'Avatar file exceeds allowed size limit', { + function: 'setUserAvatar', + url: dataURI, + sizeLimit: maxFileSize, + }); + } + + if (error instanceof Error && error.message === AVATAR_RESPONSE_TIMEOUT) { + throwAvatarDownloadTimeout(dataURI); + } + + SystemLogger.info({ + msg: 'Error while downloading avatar from provided url', + url: encodeURI(dataURI), + username: user.username, + err: error, + }); + throwAvatarUrlHandling(dataURI, user.username); + throw error; + } +}; + export const setAvatarFromServiceWithValidation = async ( userId: string, dataURI: string, @@ -175,105 +311,13 @@ export async function setUserAvatar( const { buffer, type } = await (async (): Promise<{ buffer: Buffer; type: string }> => { if (service === 'url' && typeof dataURI === 'string') { - let response: Response; - const maxFileSizeSetting = Number(settings.get('FileUpload_MaxFileSize')); - const maxFileSize = Number.isFinite(maxFileSizeSetting) && maxFileSizeSetting > 0 ? maxFileSizeSetting : DEFAULT_MAX_FILE_SIZE; - - try { - response = await fetch(dataURI, { - ignoreSsrfValidation: false, - allowList: settings.get('SSRF_Allowlist'), - timeout: DEFAULT_AVATAR_DOWNLOAD_TIMEOUT_MS, - }); - } catch (e) { - SystemLogger.info({ - msg: 'Not a valid response from the avatar url', - url: encodeURI(dataURI), - err: e, - }); - throw new Meteor.Error('error-avatar-invalid-url', `Invalid avatar URL: ${encodeURI(dataURI)}`, { + if (!user.username) { + throw new Meteor.Error('error-invalid-user', 'Invalid user', { function: 'setUserAvatar', - url: dataURI, }); } - if (response.status !== 200) { - discardResponseBody(response); - - if (response.status !== 404) { - SystemLogger.info({ - msg: 'Error while handling the setting of the avatar from a url', - url: encodeURI(dataURI), - username: user.username, - status: response.status, - }); - throw new Meteor.Error( - 'error-avatar-url-handling', - `Error while handling avatar setting from a URL (${encodeURI(dataURI)}) for ${user.username}`, - { function: 'RocketChat.setUserAvatar', url: dataURI, username: user.username }, - ); - } - - SystemLogger.info({ - msg: 'Not a valid response from the avatar url', - status: response.status, - url: dataURI, - }); - throw new Meteor.Error('error-avatar-invalid-url', `Invalid avatar URL: ${dataURI}`, { - function: 'setUserAvatar', - url: dataURI, - }); - } - - if (!/image\/.+/.test(response.headers.get('content-type') || '')) { - discardResponseBody(response); - - SystemLogger.info({ - msg: 'Not a valid content-type from the provided avatar url', - contentType: response.headers.get('content-type'), - url: dataURI, - }); - throw new Meteor.Error('error-avatar-invalid-url', `Invalid avatar URL: ${dataURI}`, { - function: 'setUserAvatar', - url: dataURI, - }); - } - - try { - return { - buffer: await bufferResponseWithLimit(response, maxFileSize, DEFAULT_AVATAR_DOWNLOAD_TIMEOUT_MS), - type: response.headers.get('content-type') || '', - }; - } catch (error) { - if (error instanceof Error && error.message === AVATAR_RESPONSE_TOO_LARGE) { - throw new Meteor.Error('error-file-too-large', 'Avatar file exceeds allowed size limit', { - function: 'setUserAvatar', - url: dataURI, - sizeLimit: maxFileSize, - }); - } - - if (error instanceof Error && error.message === AVATAR_RESPONSE_TIMEOUT) { - throw new Meteor.Error('error-avatar-download-timeout', 'Avatar download timed out', { - function: 'setUserAvatar', - url: dataURI, - timeoutMs: DEFAULT_AVATAR_DOWNLOAD_TIMEOUT_MS, - }); - } - - SystemLogger.info({ - msg: 'Error while downloading avatar from provided url', - url: encodeURI(dataURI), - username: user.username, - err: error, - }); - - throw new Meteor.Error( - 'error-avatar-url-handling', - `Error while handling avatar setting from a URL (${encodeURI(dataURI)}) for ${user.username}`, - { function: 'RocketChat.setUserAvatar', url: dataURI, username: user.username }, - ); - } + return getAvatarFromUrl({ username: user.username }, dataURI); } if (service === 'rest') { diff --git a/apps/meteor/tests/unit/app/lib/server/functions/setUserAvatar.spec.ts b/apps/meteor/tests/unit/app/lib/server/functions/setUserAvatar.spec.ts index c3cbd397749dd..e899d2fb285fb 100644 --- a/apps/meteor/tests/unit/app/lib/server/functions/setUserAvatar.spec.ts +++ b/apps/meteor/tests/unit/app/lib/server/functions/setUserAvatar.spec.ts @@ -157,6 +157,20 @@ describe('setUserAvatar', () => { expect(fileStore.insert.called).to.be.false; }); + it('rejects content types that only contain image slash in parameters', async () => { + stubs.fetch.resolves( + createResponse({ + headers: { + 'content-type': 'text/plain; note=image/png', + }, + body: Readable.from([Buffer.from('ok')]), + }), + ); + + await expect(setUserAvatar(user, 'https://example.com/avatar.txt', '', 'url')).to.be.rejectedWith('error-avatar-invalid-url'); + expect(fileStore.insert.called).to.be.false; + }); + it('stores avatar when streamed image stays within limit', async () => { stubs.fetch.resolves( createResponse({ @@ -233,4 +247,13 @@ describe('setUserAvatar', () => { expect(fileStore.insert.called).to.be.false; clock.restore(); }); + + it('maps fetch abort errors to avatar download timeout', async () => { + const abortError = Object.assign(new Error('The operation was aborted'), { name: 'AbortError' }); + + stubs.fetch.rejects(abortError); + + await expect(setUserAvatar(user, 'https://example.com/avatar.png', '', 'url')).to.be.rejectedWith('error-avatar-download-timeout'); + expect(fileStore.insert.called).to.be.false; + }); }); From cef0ddab291f32626fa7490e73f3b32b56e8ba5c Mon Sep 17 00:00:00 2001 From: Soumajit Ghosh Date: Tue, 14 Apr 2026 09:19:51 +0530 Subject: [PATCH 4/7] fix: encode avatar URL in error logging for invalid responses --- apps/meteor/app/lib/server/functions/setUserAvatar.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/app/lib/server/functions/setUserAvatar.ts b/apps/meteor/app/lib/server/functions/setUserAvatar.ts index 9af3265eb1c48..7fb8ba977157a 100644 --- a/apps/meteor/app/lib/server/functions/setUserAvatar.ts +++ b/apps/meteor/app/lib/server/functions/setUserAvatar.ts @@ -179,7 +179,7 @@ const getAvatarFromUrl = async ( SystemLogger.info({ msg: 'Not a valid response from the avatar url', status: response.status, - url: dataURI, + url: encodeURI(dataURI), }); throwInvalidAvatarUrl(dataURI); } @@ -191,7 +191,7 @@ const getAvatarFromUrl = async ( SystemLogger.info({ msg: 'Not a valid content-type from the provided avatar url', contentType: type, - url: dataURI, + url: encodeURI(dataURI), }); throwInvalidAvatarUrl(dataURI); } From 8336f5a3437e9307dfb88ae29351e7c36aebc4f1 Mon Sep 17 00:00:00 2001 From: Soumajit Ghosh Date: Tue, 14 Apr 2026 10:02:46 +0530 Subject: [PATCH 5/7] fix: redact sensitive information from avatar URL logging --- .../app/lib/server/functions/setUserAvatar.ts | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/apps/meteor/app/lib/server/functions/setUserAvatar.ts b/apps/meteor/app/lib/server/functions/setUserAvatar.ts index 7fb8ba977157a..28207b083a6ea 100644 --- a/apps/meteor/app/lib/server/functions/setUserAvatar.ts +++ b/apps/meteor/app/lib/server/functions/setUserAvatar.ts @@ -39,6 +39,23 @@ const isRequestTimeoutError = (error: unknown): boolean => { return name === 'AbortError' || code === 'ETIMEDOUT' || type === 'request-timeout'; }; +const redactUrl = (url: string): string => { + try { + const parsed = new URL(url); + parsed.username = ''; + parsed.password = ''; + parsed.searchParams.forEach((_value, key) => { + if (/token|key|secret|password|auth|session|sid/i.test(key)) { + parsed.searchParams.set(key, '[redacted]'); + } + }); + + return parsed.toString(); + } catch { + return url; + } +}; + const getMaxFileSize = (): number => { const maxFileSizeSetting = Number(settings.get('FileUpload_MaxFileSize')); @@ -124,7 +141,7 @@ const bufferResponseWithLimit = async (response: Response, sizeLimit: number, ti const onError = (error: Error): void => { cleanup(); - reject(error); + reject(isRequestTimeoutError(error) ? new Error(AVATAR_RESPONSE_TIMEOUT) : error); }; const timeoutId = setTimeout(() => rejectWithError(new Error(AVATAR_RESPONSE_TIMEOUT)), timeoutMs); @@ -155,7 +172,7 @@ const getAvatarFromUrl = async ( SystemLogger.info({ msg: 'Not a valid response from the avatar url', - url: encodeURI(dataURI), + url: redactUrl(dataURI), err: error, }); throwInvalidAvatarUrl(dataURI); @@ -169,7 +186,7 @@ const getAvatarFromUrl = async ( if (response.status !== 404) { SystemLogger.info({ msg: 'Error while handling the setting of the avatar from a url', - url: encodeURI(dataURI), + url: redactUrl(dataURI), username: user.username, status: response.status, }); @@ -179,7 +196,7 @@ const getAvatarFromUrl = async ( SystemLogger.info({ msg: 'Not a valid response from the avatar url', status: response.status, - url: encodeURI(dataURI), + url: redactUrl(dataURI), }); throwInvalidAvatarUrl(dataURI); } @@ -191,7 +208,7 @@ const getAvatarFromUrl = async ( SystemLogger.info({ msg: 'Not a valid content-type from the provided avatar url', contentType: type, - url: encodeURI(dataURI), + url: redactUrl(dataURI), }); throwInvalidAvatarUrl(dataURI); } @@ -216,7 +233,7 @@ const getAvatarFromUrl = async ( SystemLogger.info({ msg: 'Error while downloading avatar from provided url', - url: encodeURI(dataURI), + url: redactUrl(dataURI), username: user.username, err: error, }); From eccf3397ad387cd5b4d7f48773f926596ff198f6 Mon Sep 17 00:00:00 2001 From: Soumajit Ghosh Date: Tue, 14 Apr 2026 10:08:21 +0530 Subject: [PATCH 6/7] fix: streamline avatar URL fetching and improve error handling --- .../app/lib/server/functions/setUserAvatar.ts | 37 +++++++++---------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/apps/meteor/app/lib/server/functions/setUserAvatar.ts b/apps/meteor/app/lib/server/functions/setUserAvatar.ts index 28207b083a6ea..deec357e8b85c 100644 --- a/apps/meteor/app/lib/server/functions/setUserAvatar.ts +++ b/apps/meteor/app/lib/server/functions/setUserAvatar.ts @@ -158,27 +158,26 @@ const getAvatarFromUrl = async ( dataURI: string, ): Promise<{ buffer: Buffer; type: string }> => { const maxFileSize = getMaxFileSize(); - const response = await (async (): Promise => { - try { - return await fetch(dataURI, { - ignoreSsrfValidation: false, - allowList: settings.get('SSRF_Allowlist'), - timeout: DEFAULT_AVATAR_DOWNLOAD_TIMEOUT_MS, - }); - } catch (error) { - if (isRequestTimeoutError(error)) { - throwAvatarDownloadTimeout(dataURI); - } + let response!: Response; - SystemLogger.info({ - msg: 'Not a valid response from the avatar url', - url: redactUrl(dataURI), - err: error, - }); - throwInvalidAvatarUrl(dataURI); - throw error; + try { + response = await fetch(dataURI, { + ignoreSsrfValidation: false, + allowList: settings.get('SSRF_Allowlist'), + timeout: DEFAULT_AVATAR_DOWNLOAD_TIMEOUT_MS, + }); + } catch (error) { + if (isRequestTimeoutError(error)) { + throwAvatarDownloadTimeout(dataURI); } - })(); + + SystemLogger.info({ + msg: 'Not a valid response from the avatar url', + url: redactUrl(dataURI), + err: error, + }); + throwInvalidAvatarUrl(dataURI); + } if (response.status !== 200) { discardResponseBody(response); From 273b8a30bb2389e806242813bfcc09c82e78f7a6 Mon Sep 17 00:00:00 2001 From: Soumajit Ghosh Date: Tue, 14 Apr 2026 19:34:27 +0530 Subject: [PATCH 7/7] fix: improve avatar URL fetching with size limit and timeout handling --- .../app/lib/server/functions/setUserAvatar.ts | 96 +++---------------- .../server/functions/setUserAvatar.spec.ts | 43 ++++----- 2 files changed, 30 insertions(+), 109 deletions(-) diff --git a/apps/meteor/app/lib/server/functions/setUserAvatar.ts b/apps/meteor/app/lib/server/functions/setUserAvatar.ts index deec357e8b85c..f31039fbb47f7 100644 --- a/apps/meteor/app/lib/server/functions/setUserAvatar.ts +++ b/apps/meteor/app/lib/server/functions/setUserAvatar.ts @@ -1,5 +1,3 @@ -import type { Readable } from 'stream'; - import { api } from '@rocket.chat/core-services'; import type { IUser } from '@rocket.chat/core-typings'; import type { Updater } from '@rocket.chat/models'; @@ -18,8 +16,6 @@ import { settings } from '../../../settings/server'; const DEFAULT_AVATAR_DOWNLOAD_TIMEOUT_MS = 20_000; const DEFAULT_MAX_FILE_SIZE = 104_857_600; -const AVATAR_RESPONSE_TOO_LARGE = 'error-avatar-response-too-large'; -const AVATAR_RESPONSE_TIMEOUT = 'error-avatar-response-timeout'; const getMediaType = (contentTypeHeader: string | null): string => { const mediaType = contentTypeHeader?.split(';', 1)[0].trim().toLowerCase(); @@ -36,7 +32,15 @@ const isRequestTimeoutError = (error: unknown): boolean => { const { name, code, type } = error as { name?: string; code?: string; type?: string }; - return name === 'AbortError' || code === 'ETIMEDOUT' || type === 'request-timeout'; + return name === 'AbortError' || code === 'ETIMEDOUT' || type === 'request-timeout' || type === 'body-timeout'; +}; + +const isResponseTooLargeError = (error: unknown): boolean => { + if (!error || typeof error !== 'object') { + return false; + } + + return (error as { type?: string }).type === 'max-size'; }; const redactUrl = (url: string): string => { @@ -85,74 +89,6 @@ const throwAvatarUrlHandling = (dataURI: string, username: string): never => { ); }; -const discardResponseBody = (response: Pick): void => { - const body = response.body as Partial | null; - - body?.resume?.(); - body?.destroy?.(); -}; - -const bufferResponseWithLimit = async (response: Response, sizeLimit: number, timeoutMs: number): Promise => { - const contentLength = Number.parseInt(response.headers.get('content-length') || '', 10); - - if (Number.isFinite(contentLength) && contentLength > sizeLimit) { - discardResponseBody(response); - throw new Error(AVATAR_RESPONSE_TOO_LARGE); - } - - if (!response.body) { - return Buffer.alloc(0); - } - - return new Promise((resolve, reject) => { - const body = response.body as Readable; - const chunks: Buffer[] = []; - let totalBytes = 0; - - const cleanup = (): void => { - clearTimeout(timeoutId); - body.removeListener('data', onData); - body.removeListener('end', onEnd); - body.removeListener('error', onError); - }; - - const rejectWithError = (error: Error): void => { - cleanup(); - body.destroy(); - reject(error); - }; - - const onData = (chunk: Buffer | string): void => { - const bufferChunk = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); - totalBytes += bufferChunk.length; - - if (totalBytes > sizeLimit) { - rejectWithError(new Error(AVATAR_RESPONSE_TOO_LARGE)); - return; - } - - chunks.push(bufferChunk); - }; - - const onEnd = (): void => { - cleanup(); - resolve(Buffer.concat(chunks)); - }; - - const onError = (error: Error): void => { - cleanup(); - reject(isRequestTimeoutError(error) ? new Error(AVATAR_RESPONSE_TIMEOUT) : error); - }; - - const timeoutId = setTimeout(() => rejectWithError(new Error(AVATAR_RESPONSE_TIMEOUT)), timeoutMs); - - body.on('data', onData); - body.once('end', onEnd); - body.once('error', onError); - body.resume(); - }); -}; - const getAvatarFromUrl = async ( user: Pick & { username: string }, dataURI: string, @@ -164,6 +100,7 @@ const getAvatarFromUrl = async ( response = await fetch(dataURI, { ignoreSsrfValidation: false, allowList: settings.get('SSRF_Allowlist'), + size: maxFileSize, timeout: DEFAULT_AVATAR_DOWNLOAD_TIMEOUT_MS, }); } catch (error) { @@ -180,8 +117,6 @@ const getAvatarFromUrl = async ( } if (response.status !== 200) { - discardResponseBody(response); - if (response.status !== 404) { SystemLogger.info({ msg: 'Error while handling the setting of the avatar from a url', @@ -202,8 +137,6 @@ const getAvatarFromUrl = async ( const type = response.headers.get('content-type') || ''; if (!isImageContentType(type)) { - discardResponseBody(response); - SystemLogger.info({ msg: 'Not a valid content-type from the provided avatar url', contentType: type, @@ -214,11 +147,11 @@ const getAvatarFromUrl = async ( try { return { - buffer: await bufferResponseWithLimit(response, maxFileSize, DEFAULT_AVATAR_DOWNLOAD_TIMEOUT_MS), + buffer: Buffer.from(await response.arrayBuffer()), type, }; } catch (error) { - if (error instanceof Error && error.message === AVATAR_RESPONSE_TOO_LARGE) { + if (isResponseTooLargeError(error)) { throw new Meteor.Error('error-file-too-large', 'Avatar file exceeds allowed size limit', { function: 'setUserAvatar', url: dataURI, @@ -226,7 +159,7 @@ const getAvatarFromUrl = async ( }); } - if (error instanceof Error && error.message === AVATAR_RESPONSE_TIMEOUT) { + if (isRequestTimeoutError(error)) { throwAvatarDownloadTimeout(dataURI); } @@ -236,8 +169,7 @@ const getAvatarFromUrl = async ( username: user.username, err: error, }); - throwAvatarUrlHandling(dataURI, user.username); - throw error; + return throwAvatarUrlHandling(dataURI, user.username); } }; diff --git a/apps/meteor/tests/unit/app/lib/server/functions/setUserAvatar.spec.ts b/apps/meteor/tests/unit/app/lib/server/functions/setUserAvatar.spec.ts index e899d2fb285fb..92dbfc6565b9c 100644 --- a/apps/meteor/tests/unit/app/lib/server/functions/setUserAvatar.spec.ts +++ b/apps/meteor/tests/unit/app/lib/server/functions/setUserAvatar.spec.ts @@ -1,5 +1,3 @@ -import { Readable } from 'stream'; - import { expect } from 'chai'; import proxyquire from 'proxyquire'; import sinon from 'sinon'; @@ -17,17 +15,17 @@ class MeteorError extends Error { const createResponse = ({ status = 200, headers = {}, - body, + arrayBuffer, }: { status?: number; headers?: Record; - body?: Readable | null; + arrayBuffer?: sinon.SinonStub; }) => ({ status, headers: { get: (key: string) => headers[key.toLowerCase()] || null, }, - body: body ?? null, + arrayBuffer: arrayBuffer ?? sinon.stub().resolves(Buffer.from('1234')), }); describe('setUserAvatar', () => { @@ -104,9 +102,8 @@ describe('setUserAvatar', () => { createResponse({ headers: { 'content-type': 'image/png', - 'content-length': '10', }, - body: Readable.from([Buffer.from('1234')]), + arrayBuffer: sinon.stub().rejects(Object.assign(new Error('too large'), { type: 'max-size' })), }), ); @@ -114,13 +111,13 @@ describe('setUserAvatar', () => { expect(fileStore.insert.called).to.be.false; }); - it('rejects avatar url when streamed body exceeds max size without content-length', async () => { + it('rejects avatar url when response body exceeds max size', async () => { stubs.fetch.resolves( createResponse({ headers: { 'content-type': 'image/png', }, - body: Readable.from([Buffer.from('12'), Buffer.from('345')]), + arrayBuffer: sinon.stub().rejects(Object.assign(new Error('too large'), { type: 'max-size' })), }), ); @@ -128,14 +125,14 @@ describe('setUserAvatar', () => { expect(fileStore.insert.called).to.be.false; }); - it('rejects avatar url when body exceeds lied content-length', async () => { + it('rejects avatar url when response exceeds lied content-length', async () => { stubs.fetch.resolves( createResponse({ headers: { 'content-type': 'image/png', 'content-length': '2', }, - body: Readable.from([Buffer.from('12345')]), + arrayBuffer: sinon.stub().rejects(Object.assign(new Error('too large'), { type: 'max-size' })), }), ); @@ -149,7 +146,6 @@ describe('setUserAvatar', () => { headers: { 'content-type': 'text/plain', }, - body: Readable.from([Buffer.from('ok')]), }), ); @@ -163,7 +159,6 @@ describe('setUserAvatar', () => { headers: { 'content-type': 'text/plain; note=image/png', }, - body: Readable.from([Buffer.from('ok')]), }), ); @@ -176,9 +171,8 @@ describe('setUserAvatar', () => { createResponse({ headers: { 'content-type': 'image/png', - 'content-length': '4', }, - body: Readable.from([Buffer.from('12'), Buffer.from('34')]), + arrayBuffer: sinon.stub().resolves(Buffer.from('1234')), }), ); @@ -191,6 +185,10 @@ describe('setUserAvatar', () => { size: 4, }); expect(fileStore.insert.firstCall.args[1].equals(Buffer.from('1234'))).to.be.true; + expect(stubs.fetch.firstCall.args[1]).to.deep.include({ + size: 4, + timeout: 20_000, + }); }); it('falls back to default max file size when setting is negative', async () => { @@ -210,9 +208,8 @@ describe('setUserAvatar', () => { createResponse({ headers: { 'content-type': 'image/png', - 'content-length': '10', }, - body: Readable.from([Buffer.from('1234567890')]), + arrayBuffer: sinon.stub().resolves(Buffer.from('1234567890')), }), ); @@ -227,25 +224,17 @@ describe('setUserAvatar', () => { }); it('rejects avatar url when body read times out', async () => { - const clock = sinon.useFakeTimers(); - const hangingStream = new Readable({ read: sinon.stub() }); - stubs.fetch.resolves( createResponse({ headers: { 'content-type': 'image/png', }, - body: hangingStream, + arrayBuffer: sinon.stub().rejects(Object.assign(new Error('Response timeout'), { type: 'body-timeout' })), }), ); - const avatarPromise = setUserAvatar(user, 'https://example.com/avatar.png', '', 'url'); - const assertion = expect(avatarPromise).to.be.rejectedWith('error-avatar-download-timeout'); - await clock.tickAsync(20_001); - - await assertion; + await expect(setUserAvatar(user, 'https://example.com/avatar.png', '', 'url')).to.be.rejectedWith('error-avatar-download-timeout'); expect(fileStore.insert.called).to.be.false; - clock.restore(); }); it('maps fetch abort errors to avatar download timeout', async () => {