Skip to content

Commit

Permalink
fix(next): reject protocol-relative URLs in image optimization (#65752)
Browse files Browse the repository at this point in the history
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 <steven@ceriously.com>
  • Loading branch information
2 people authored and huozhi committed Jul 9, 2024
1 parent c64c61d commit 1b10b13
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
5 changes: 5 additions & 0 deletions test/integration/image-optimizer/app/pages/api/no-header.js
Original file line number Diff line number Diff line change
@@ -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')
}
2 changes: 1 addition & 1 deletion test/integration/image-optimizer/test/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,7 @@ export function runTests(ctx) {
})

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)
Expand Down

0 comments on commit 1b10b13

Please sign in to comment.