fix(next): reject protocol-relative URLs in image optimization (verce…

…l#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 <[email protected]>

packages/next/src/server/image-optimizer.ts

@@ -198,6 +198,12 @@ export class ImageOptimizerCache {
       return { errorMessage: '"url" parameter is too long' }
     }
 
+    if (url.startsWith('//')) {
+      return {
+        errorMessage: '"url" parameter cannot be a protocol-relative URL (//)',
+      }
+    }
+
     let isAbsolute: boolean
 
     if (url.startsWith('/')) {

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')
+}

test/integration/image-optimizer/test/util.ts

@@ -987,8 +987,17 @@ export function runTests(ctx: RunTestsCtx) {
     expect(await res.text()).toBe(`"url" parameter is too long`)
   })
 
+  it('should fail when url is protocol relative', async () => {
+    const query = { url: `//example.com`, 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 a protocol-relative URL (//)`
+    )
+  })
+
   it('should fail when internal url is not an image', async () => {
-    const url = `//<h1>not-an-image</h1>`
+    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)