From c7f38d96213a418b78f5587a22fee7f2010a0247 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 3 May 2026 22:16:40 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20fix:=20Validate=20Avata?= =?UTF-8?q?r=20URL=20Before=20Fetch=20(#12928)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resizeAvatar` previously called `node-fetch` on any string input with no validation. When OIDC providers surface a user-controllable `picture` claim, this could be used to make blind SSRF requests to internal services on every social login. Wrap the URL fetch with: - An allowlist on the URL protocol (http/https only). - The shared `createSSRFSafeAgents` utility, which blocks resolution to private, loopback, and link-local IPs at TCP connect time (TOCTOU-safe; works equally for hostname targets that DNS-resolve privately and for IP-literal targets, since Node's `net.Socket` always dispatches through the agent's `lookup` hook). - `redirect: 'error'` so a public-IP redirect target cannot be used to bypass the agent check on a subsequent hop. - A 5-second total request budget (node-fetch v2's `timeout` covers request initiation through full body receipt, bounding slow-loris exposure rather than just the TCP connect). - A 10 MB response cap (`size` option + `Content-Length` pre-check + post-read length assertion) so a hostile payload cannot exhaust memory before `sharp()` rejects it. Fetch the canonicalized `parsed.href` rather than the raw input string to eliminate any future parser-differential between `new URL()` and the underlying fetch implementation. Per-call agent construction is intentional: the avatar path runs once per social login per user, so pooling adds complexity without a measurable benefit. Documented inline. Comprehensive test coverage in `avatar.spec.js`: - Rejects malformed URLs, non-http(s) schemes (file://, data:, javascript:). - Asserts the happy-path canonicalization (`fetch` is called with `parsed.href`) and the SSRF-safe agent factory routing (https→httpsAgent, http→httpAgent). - Rejects non-2xx HTTP status. - Rejects an oversized Content-Length before reading the body, and asserts `.buffer()` is never invoked in that case. - Rejects an oversized body even when the server lies about / omits Content-Length. - Surfaces ESSRF, redirect, and `size` overflow errors thrown by the fetch layer. - Confirms Buffer inputs bypass the fetcher entirely. --- api/server/services/Files/images/avatar.js | 73 ++++++- .../services/Files/images/avatar.spec.js | 189 ++++++++++++++++++ 2 files changed, 256 insertions(+), 6 deletions(-) create mode 100644 api/server/services/Files/images/avatar.spec.js diff --git a/api/server/services/Files/images/avatar.js b/api/server/services/Files/images/avatar.js index 9ce11c04ec..640aed293f 100644 --- a/api/server/services/Files/images/avatar.js +++ b/api/server/services/Files/images/avatar.js @@ -3,8 +3,74 @@ const fs = require('fs').promises; const fetch = require('node-fetch'); const { logger } = require('@librechat/data-schemas'); const { EImageOutputType } = require('librechat-data-provider'); +const { createSSRFSafeAgents } = require('@librechat/api'); const { resizeAndConvert } = require('./resize'); +const ALLOWED_AVATAR_PROTOCOLS = new Set(['http:', 'https:']); +/** + * Cap response size to bound memory exposure if a malicious or compromised + * `picture` URL serves a multi-GB payload. Avatars are at most a few hundred + * KB in practice; 10 MB is well past any legitimate use. + */ +const MAX_AVATAR_BYTES = 10 * 1024 * 1024; + +/** + * Fetches an image URL with SSRF protection: rejects non-http(s) schemes, + * blocks resolution to private/loopback/link-local IPs at TCP connect time, + * refuses to follow redirects to prevent post-validation rebinding, and caps + * the response body so a hostile payload cannot exhaust memory before + * `sharp()` rejects it. + * + * Per-call agent construction is intentional: avatar fetches are infrequent + * (once per social login per user) and pooling adds complexity without a + * measurable benefit on this path. If this ever becomes a hot path, hoist + * the agents to module scope. + */ +async function fetchAvatarBuffer(input) { + let parsed; + try { + parsed = new URL(input); + } catch { + throw new Error('Invalid avatar URL'); + } + if (!ALLOWED_AVATAR_PROTOCOLS.has(parsed.protocol)) { + throw new Error(`Refusing to fetch avatar over ${parsed.protocol}`); + } + + const { httpAgent, httpsAgent } = createSSRFSafeAgents(); + /** + * `node-fetch` v2's `timeout` is the total request budget (request initiation + * through full body receipt), not a TCP-connect-only timeout. That is the + * stronger of the two for this path — bounds total slow-loris exposure. + */ + const response = await fetch(parsed.href, { + agent: (urlObj) => (urlObj.protocol === 'https:' ? httpsAgent : httpAgent), + redirect: 'error', + timeout: 5000, + size: MAX_AVATAR_BYTES, + }); + + if (!response.ok) { + throw new Error(`Failed to fetch image from URL. Status: ${response.status}`); + } + + const contentLength = parseInt(response.headers.get('content-length') ?? '0', 10); + if (contentLength > MAX_AVATAR_BYTES) { + throw new Error(`Avatar response too large: ${contentLength} bytes`); + } + + /** + * Re-check after read in case the server lied about Content-Length or + * omitted it. `node-fetch` v2 honors the `size` option above and throws on + * overflow, but Defense-in-depth: assert on the actual buffer length. + */ + const buffer = await response.buffer(); + if (buffer.length > MAX_AVATAR_BYTES) { + throw new Error(`Avatar response too large: ${buffer.length} bytes`); + } + return buffer; +} + /** * Uploads an avatar image for a user. This function can handle various types of input (URL, Buffer, or File object), * processes the image to a square format, converts it to target format, and returns the resized buffer. @@ -29,12 +95,7 @@ async function resizeAvatar({ userId, input, desiredFormat = EImageOutputType.PN let imageBuffer; if (typeof input === 'string') { - const response = await fetch(input); - - if (!response.ok) { - throw new Error(`Failed to fetch image from URL. Status: ${response.status}`); - } - imageBuffer = await response.buffer(); + imageBuffer = await fetchAvatarBuffer(input); } else if (input instanceof Buffer) { imageBuffer = input; } else if (typeof input === 'object' && input instanceof File) { diff --git a/api/server/services/Files/images/avatar.spec.js b/api/server/services/Files/images/avatar.spec.js new file mode 100644 index 0000000000..cc3e0b1ad1 --- /dev/null +++ b/api/server/services/Files/images/avatar.spec.js @@ -0,0 +1,189 @@ +/** + * Tests for the SSRF-safe avatar fetcher in `avatar.js`. + * + * The function is the sole line of defense against SSRF when a social + * login surfaces a user-controllable `picture` URL. We assert each + * rejection branch (protocol, status, redirect, size, agent) and the + * happy path so that a future refactor of the fetch / agent / URL + * handling cannot silently break the protection. + */ +jest.mock('node-fetch'); +jest.mock('@librechat/api', () => ({ + createSSRFSafeAgents: jest.fn(() => ({ + httpAgent: { __kind: 'http' }, + httpsAgent: { __kind: 'https' }, + })), +})); +jest.mock('@librechat/data-schemas', () => ({ + logger: { info: jest.fn(), warn: jest.fn(), error: jest.fn(), debug: jest.fn() }, +})); +jest.mock('librechat-data-provider', () => ({ + EImageOutputType: { PNG: 'png' }, +})); +jest.mock('./resize', () => ({ + resizeAndConvert: jest.fn(async ({ inputBuffer }) => ({ buffer: inputBuffer })), +})); +jest.mock('sharp', () => { + const sharpFn = jest.fn(); + return sharpFn; +}); + +const fetch = require('node-fetch'); +const { createSSRFSafeAgents } = require('@librechat/api'); +const sharp = require('sharp'); +const { resizeAvatar } = require('./avatar'); + +function makeResponse({ ok = true, status = 200, body = Buffer.from(''), contentLength } = {}) { + return { + ok, + status, + headers: { + get: (name) => { + if (name.toLowerCase() === 'content-length') { + return contentLength != null ? String(contentLength) : null; + } + return null; + }, + }, + buffer: jest.fn(async () => body), + }; +} + +function makeSharpStub(format = 'png', width = 100, height = 100) { + const chain = { + metadata: jest.fn(async () => ({ format, width, height })), + extract: jest.fn(() => chain), + resize: jest.fn(() => chain), + gif: jest.fn(() => chain), + toBuffer: jest.fn(async () => Buffer.from('squared')), + }; + return chain; +} + +const callResize = (input) => resizeAvatar({ userId: 'u1', input }); + +describe('resizeAvatar — fetchAvatarBuffer', () => { + beforeEach(() => { + jest.clearAllMocks(); + sharp.mockImplementation(() => makeSharpStub()); + }); + + describe('rejects unsafe inputs before any network call', () => { + it('rejects a malformed URL string', async () => { + await expect(callResize('not-a-url')).rejects.toThrow('Invalid avatar URL'); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('rejects file:// URLs', async () => { + await expect(callResize('file:///etc/passwd')).rejects.toThrow(/Refusing to fetch.*file:/); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('rejects data: URLs', async () => { + await expect(callResize('data:image/png;base64,AAAA')).rejects.toThrow( + /Refusing to fetch.*data:/, + ); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('rejects javascript: URLs', async () => { + await expect(callResize('javascript:void(0)')).rejects.toThrow( + /Refusing to fetch.*javascript:/, + ); + expect(fetch).not.toHaveBeenCalled(); + }); + }); + + describe('happy path', () => { + it('returns a processed buffer for a valid https URL', async () => { + fetch.mockResolvedValueOnce(makeResponse({ body: Buffer.from('rawimg') })); + const result = await callResize('https://cdn.example.com/avatar.png'); + expect(fetch).toHaveBeenCalledTimes(1); + // `parsed.href` canonicalizes the input — assert we did not pass the raw string. + expect(fetch.mock.calls[0][0]).toBe('https://cdn.example.com/avatar.png'); + const opts = fetch.mock.calls[0][1]; + expect(opts.redirect).toBe('error'); + expect(opts.timeout).toBe(5000); + expect(opts.size).toBe(10 * 1024 * 1024); + expect(typeof opts.agent).toBe('function'); + expect(result).toEqual(Buffer.from('squared')); + }); + + it('passes an SSRF-safe agent factory routing https→httpsAgent and http→httpAgent', async () => { + fetch.mockResolvedValueOnce(makeResponse({ body: Buffer.from('rawimg') })); + await callResize('https://cdn.example.com/avatar.png'); + const agentFn = fetch.mock.calls[0][1].agent; + expect(agentFn(new URL('https://anything'))).toEqual({ __kind: 'https' }); + expect(agentFn(new URL('http://anything'))).toEqual({ __kind: 'http' }); + expect(createSSRFSafeAgents).toHaveBeenCalledTimes(1); + }); + }); + + describe('rejects unsafe responses', () => { + it('rejects non-2xx HTTP status', async () => { + fetch.mockResolvedValueOnce(makeResponse({ ok: false, status: 500 })); + await expect(callResize('https://cdn.example.com/avatar.png')).rejects.toThrow( + /Status:\s*500/, + ); + }); + + it('rejects an oversized Content-Length header before reading the body', async () => { + const oversize = 11 * 1024 * 1024; + const resp = makeResponse({ contentLength: oversize }); + fetch.mockResolvedValueOnce(resp); + await expect(callResize('https://cdn.example.com/big.png')).rejects.toThrow( + /Avatar response too large.*11534336/, + ); + // We must not even read the body once the header has already disqualified it. + expect(resp.buffer).not.toHaveBeenCalled(); + }); + + it('rejects a body whose actual size exceeds the cap (lying / missing Content-Length)', async () => { + const oversize = Buffer.alloc(11 * 1024 * 1024); + // No content-length header — server lies or omits. + fetch.mockResolvedValueOnce(makeResponse({ body: oversize })); + await expect(callResize('https://cdn.example.com/lies.png')).rejects.toThrow( + /Avatar response too large.*11534336/, + ); + }); + }); + + describe('propagates fetch-layer errors', () => { + it('surfaces SSRF rejection thrown by the agent (ESSRF)', async () => { + const ssrfError = Object.assign(new Error('SSRF protection: 127.0.0.1 blocked'), { + code: 'ESSRF', + }); + fetch.mockRejectedValueOnce(ssrfError); + await expect(callResize('http://internal.attacker.example/img.png')).rejects.toThrow( + /SSRF protection/, + ); + }); + + it('surfaces redirect rejection from `redirect: error`', async () => { + const redirectError = Object.assign(new Error('redirect mode is set to error'), { + type: 'no-redirect', + }); + fetch.mockRejectedValueOnce(redirectError); + await expect(callResize('https://cdn.example.com/redirected.png')).rejects.toThrow( + /redirect mode/, + ); + }); + + it('surfaces a `size` overflow thrown by node-fetch', async () => { + const sizeError = Object.assign(new Error('content size at 11534336 over limit: 10485760'), { + type: 'max-size', + }); + fetch.mockRejectedValueOnce(sizeError); + await expect(callResize('https://cdn.example.com/large.png')).rejects.toThrow(/over limit/); + }); + }); + + describe('non-string inputs bypass the fetcher', () => { + it('accepts a Buffer input directly without calling fetch', async () => { + const buf = Buffer.from('inline'); + const result = await callResize(buf); + expect(fetch).not.toHaveBeenCalled(); + expect(result).toEqual(Buffer.from('squared')); + }); + }); +});