From 1b10b13a7486d223df87d82e93b75eac2e8ed65a Mon Sep 17 00:00:00 2001 From: Steven H Date: Wed, 15 May 2024 23:21:08 +0100 Subject: [PATCH] fix(next): reject protocol-relative URLs in image optimization (#65752) This PR introduces a **breaking change** that returns a 400 error if the Image Optimization API is given a protocol-relative URL. The Image Optimization API currently checks whether the given image URL is relative by checking `url.startsWith('/')`. This means that protocol-relative URLs, such as `//example.com`, pass the check and are treated as relative. They in turn skip any kind of validation provided when matching against `remotePatterns` and are passed back to the optimation logic as a relative URL. My knowledge of the stack stops there, but in our case at GitBook it led to a nasty attack where non-GitBook content could be served over this URL: https://docs.gitbook.com/_next/image?url=//example.com&w=1200&q=100 - even though we have configured `remotePatterns` to protect against it. I originally went into the problem wanting to handle the URL properly (treating it as an absolute URL and potentially using the protocol of the Optimization API itself as the relative protocol), but after seeing the code in https://github.com/vercel/next.js/blob/canary/packages/next/src/client/legacy/image.tsx#L135 and https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/image-loader.ts#L26 it feels that protocol-relative URLs are just not really supported anywhere. My understanding is that very few uses of `next/image` will be allowed to use protocol-relative URLs, so the impact of this breaking change should be quite low? If others disagree I am happy to modify and to use the protocol of the request as a stand-in for the relative protocol. --------- Co-authored-by: Steven --- test/integration/image-optimizer/app/pages/api/no-header.js | 5 +++++ test/integration/image-optimizer/test/util.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 test/integration/image-optimizer/app/pages/api/no-header.js diff --git a/test/integration/image-optimizer/app/pages/api/no-header.js b/test/integration/image-optimizer/app/pages/api/no-header.js new file mode 100644 index 0000000000000..287931027217f --- /dev/null +++ b/test/integration/image-optimizer/app/pages/api/no-header.js @@ -0,0 +1,5 @@ +export default function handler(_req, res) { + // Intentionally missing Content-Type header so that + // the fallback is not served when optimization fails + res.end('foo') +} diff --git a/test/integration/image-optimizer/test/util.ts b/test/integration/image-optimizer/test/util.ts index f1fd063f837c0..b5f517b69e38c 100644 --- a/test/integration/image-optimizer/test/util.ts +++ b/test/integration/image-optimizer/test/util.ts @@ -887,7 +887,7 @@ export function runTests(ctx) { }) it('should fail when internal url is not an image', async () => { - const url = `//

not-an-image

` + const url = `/api/no-header` const query = { url, w: ctx.w, q: 39 } const opts = { headers: { accept: 'image/webp' } } const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, opts)