From 792e4afaa228ed51a5c49f6a91b6b56d6dad60cb Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 7 Aug 2024 18:28:50 +0200 Subject: [PATCH 1/2] Reject next image urls in image optimizer --- packages/next/src/lib/url.ts | 8 ++++ packages/next/src/server/image-optimizer.ts | 12 +++-- test/integration/image-optimizer/test/util.ts | 46 +++++++++++++++++-- test/lib/next-test-utils.ts | 9 ++++ 4 files changed, 67 insertions(+), 8 deletions(-) diff --git a/packages/next/src/lib/url.ts b/packages/next/src/lib/url.ts index f5969c1019449..cc0b37d23c365 100644 --- a/packages/next/src/lib/url.ts +++ b/packages/next/src/lib/url.ts @@ -6,6 +6,14 @@ export function isFullStringUrl(url: string) { return /https?:\/\//.test(url) } +export function parseUrl(url: string): URL | undefined { + let parsed = undefined + try { + parsed = new URL(url, DUMMY_ORIGIN) + } catch {} + return parsed +} + export function stripNextRscUnionQuery(relativeUrl: string): string { const urlInstance = new URL(relativeUrl, DUMMY_ORIGIN) urlInstance.searchParams.delete(NEXT_RSC_UNION_QUERY) diff --git a/packages/next/src/server/image-optimizer.ts b/packages/next/src/server/image-optimizer.ts index 4dfd153e848f9..56e41ebbc9ab3 100644 --- a/packages/next/src/server/image-optimizer.ts +++ b/packages/next/src/server/image-optimizer.ts @@ -22,6 +22,7 @@ import { sendEtagResponse } from './send-payload' import { getContentType, getExtension } from './serve-static' import * as Log from '../build/output/log' import isError from '../lib/is-error' +import { parseUrl } from '../lib/url' type XCacheHeader = 'MISS' | 'HIT' | 'STALE' @@ -213,9 +214,14 @@ export class ImageOptimizerCache { } } - if (url.startsWith('/_next/image')) { - return { - errorMessage: '"url" parameter cannot be recursive', + const parsedUrl = parseUrl(url) + if (parsedUrl) { + const decodedPathname = decodeURIComponent(parsedUrl.pathname) + console.log('decodedPathname', decodedPathname) + if (/\/_next\/image($|\/)/.test(decodedPathname)) { + return { + errorMessage: '"url" parameter cannot be recursive', + } } } diff --git a/test/integration/image-optimizer/test/util.ts b/test/integration/image-optimizer/test/util.ts index 30e9d825a9291..06fa8a16e2b1f 100644 --- a/test/integration/image-optimizer/test/util.ts +++ b/test/integration/image-optimizer/test/util.ts @@ -8,6 +8,7 @@ import { fetchViaHTTP, File, findPort, + getFetchUrl, killApp, launchApp, nextBuild, @@ -1021,11 +1022,46 @@ export function runTests(ctx: RunTestsCtx) { ) }) - it('should fail when url is recursive', async () => { - const query = { url: `/_next/image?url=test.pngw=1&q=1`, w: ctx.w, q: 1 } - const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, {}) - expect(res.status).toBe(400) - expect(await res.text()).toBe(`"url" parameter cannot be recursive`) + describe('recursive url is not allowed', () => { + it('should fail with relative next image url', async () => { + const query = { url: `/_next/image?url=test.pngw=1&q=1`, w: ctx.w, q: 1 } + const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, {}) + expect(res.status).toBe(400) + expect(await res.text()).toBe(`"url" parameter cannot be recursive`) + }) + + it('should fail with encoded relative image url', async () => { + const query = { + url: '%2F_next%2Fimage%3Furl%3Dtest.pngw%3D1%26q%3D1', + w: ctx.w, + q: 1, + } + const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, {}) + expect(res.status).toBe(400) + expect(await res.text()).toBe(`"url" parameter is invalid`) + }) + + it('should fail with absolute next image url', async () => { + const fullUrl = getFetchUrl( + ctx.appPort, + '/_next/image?url=test.pngw=1&q=1' + ) + const query = { url: fullUrl, w: ctx.w, q: 1 } + const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, {}) + expect(res.status).toBe(400) + expect(await res.text()).toBe(`"url" parameter cannot be recursive`) + }) + + it('should fail with relative image url with assetPrefix', async () => { + const fullUrl = getFetchUrl( + ctx.appPort, + `/assets/_next/image?url=test.pngw=1&q=1` + ) + const query = { url: fullUrl, w: ctx.w, q: 1 } + const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, {}) + expect(res.status).toBe(400) + expect(await res.text()).toBe(`"url" parameter cannot be recursive`) + }) }) it('should fail when internal url is not an image', async () => { diff --git a/test/lib/next-test-utils.ts b/test/lib/next-test-utils.ts index 727b71d30bc69..9aac68044d560 100644 --- a/test/lib/next-test-utils.ts +++ b/test/lib/next-test-utils.ts @@ -158,6 +158,15 @@ export function withQuery( return `${pathname}?${querystring}` } +export function getFetchUrl( + appPort: string | number, + pathname: string, + query?: Record | string | null | undefined +) { + const url = query ? withQuery(pathname, query) : pathname + return getFullUrl(appPort, url) +} + export function fetchViaHTTP( appPort: string | number, pathname: string, From 760c26c3eb37d6a3eee1d0871d53897704f5409d Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 7 Aug 2024 18:53:53 +0200 Subject: [PATCH 2/2] rm log --- packages/next/src/server/image-optimizer.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/next/src/server/image-optimizer.ts b/packages/next/src/server/image-optimizer.ts index 56e41ebbc9ab3..099076753eec8 100644 --- a/packages/next/src/server/image-optimizer.ts +++ b/packages/next/src/server/image-optimizer.ts @@ -217,7 +217,6 @@ export class ImageOptimizerCache { const parsedUrl = parseUrl(url) if (parsedUrl) { const decodedPathname = decodeURIComponent(parsedUrl.pathname) - console.log('decodedPathname', decodedPathname) if (/\/_next\/image($|\/)/.test(decodedPathname)) { return { errorMessage: '"url" parameter cannot be recursive',