Skip to content

Commit 07416ba

Browse files
matthewdavidrodgersmatthewdavidrodgers
authored
Also handle next apps hosted on a subpath (#9785)
* Also handle next apps hosted on a subpath Next apps can be hosted on a subpath, i.e. not at the root, so we need to be a little more permissive with the path we check on. * Refactor similar tests to use it.each
1 parent 671eae4 commit 07416ba

File tree

3 files changed

+100
-106
lines changed

3 files changed

+100
-106
lines changed

‎.changeset/tangy-ghosts-rule.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@cloudflare/workers-shared": patch
3+
---
4+
5+
Handle next apps hosted at a path other than the root when blocking vulnerable non-image requests

‎packages/workers-shared/router-worker/src/worker.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ export default {
113113
});
114114

115115
let shouldBlockNonImageResponse = false;
116-
if (url.pathname === "/_next/image") {
116+
if (url.pathname.endsWith("/_next/image")) {
117117
// is a next image
118118
const queryURLParam = url.searchParams.get("url");
119119
if (queryURLParam && !queryURLParam.startsWith("/")) {

‎packages/workers-shared/router-worker/tests/index.test.ts‎

Lines changed: 94 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -247,10 +247,19 @@ describe("unit tests", async () => {
247247
expect(await response.text()).toEqual("hello from user worker");
248248
});
249249

250-
it("blocks /_next/image requests with remote URLs when not fetched as image", async () => {
251-
const request = new Request(
252-
"https://example.com/_next/image?url=https://evil.com/ssrf"
253-
);
250+
it.each([
251+
{
252+
description:
253+
"blocks /_next/image requests with remote URLs when not fetched as image",
254+
url: "https://example.com/_next/image?url=https://evil.com/ssrf",
255+
},
256+
{
257+
description:
258+
"blocks /_next/image requests with remote URLs when not fetched as image for next apps host not at the root",
259+
url: "https://example.com/some/subpath/_next/image?url=https://evil.com/ssrf",
260+
},
261+
])("$description", async ({ url }) => {
262+
const request = new Request(url);
254263
const ctx = createExecutionContext();
255264

256265
const env = {
@@ -272,111 +281,91 @@ describe("unit tests", async () => {
272281
expect(await response.text()).toBe("Blocked");
273282
});
274283

275-
it("allows /_next/image requests with remote URLs when fetched as image", async () => {
276-
const request = new Request(
277-
"https://example.com/_next/image?url=https://example.com/image.jpg",
278-
{
279-
headers: { "sec-fetch-dest": "image" },
280-
}
281-
);
282-
const ctx = createExecutionContext();
283-
284-
const env = {
285-
CONFIG: {
286-
has_user_worker: true,
287-
invoke_user_worker_ahead_of_assets: true,
288-
},
289-
USER_WORKER: {
290-
async fetch(_: Request): Promise<Response> {
291-
return new Response("fake image data", {
292-
headers: { "content-type": "image/jpeg" },
293-
});
294-
},
295-
},
296-
} as Env;
297-
298-
const response = await worker.fetch(request, env, ctx);
299-
expect(response.status).toBe(200);
300-
expect(await response.text()).toBe("fake image data");
301-
});
302-
303-
it("allows /_next/image with remote URL and image header regardless of response content", async () => {
304-
const request = new Request(
305-
"https://example.com/_next/image?url=https://example.com/image.jpg",
306-
{
307-
headers: { "sec-fetch-dest": "image" },
308-
}
309-
);
310-
const ctx = createExecutionContext();
311-
312-
const env = {
313-
CONFIG: {
314-
has_user_worker: true,
315-
invoke_user_worker_ahead_of_assets: true,
316-
},
317-
USER_WORKER: {
318-
async fetch(_: Request): Promise<Response> {
319-
return new Response("<!DOCTYPE html><html></html>", {
320-
headers: { "content-type": "text/html" },
321-
});
322-
},
323-
},
324-
} as Env;
325-
326-
const response = await worker.fetch(request, env, ctx);
327-
expect(response.status).toBe(200);
328-
expect(await response.text()).toBe("<!DOCTYPE html><html></html>");
329-
});
330-
331-
it("allows /_next/image requests with local URLs", async () => {
332-
const request = new Request(
333-
"https://example.com/_next/image?url=/local/image.jpg"
334-
);
335-
const ctx = createExecutionContext();
284+
it.each([
285+
{
286+
description:
287+
"allows /_next/image requests with remote URLs when fetched as image",
288+
url: "https://example.com/_next/image?url=https://example.com/image.jpg",
289+
headers: { "sec-fetch-dest": "image" } as HeadersInit,
290+
userWorkerResponse: {
291+
body: "fake image data",
292+
headers: { "content-type": "image/jpeg" },
293+
status: 200,
294+
},
295+
expectedStatus: 200,
296+
expectedBody: "fake image data",
297+
},
298+
{
299+
description:
300+
"allows /_next/image with remote URL and image header regardless of response content",
301+
url: "https://example.com/_next/image?url=https://example.com/image.jpg",
302+
headers: { "sec-fetch-dest": "image" } as HeadersInit,
303+
userWorkerResponse: {
304+
body: "<!DOCTYPE html><html></html>",
305+
headers: { "content-type": "text/html" },
306+
status: 200,
307+
},
308+
expectedStatus: 200,
309+
expectedBody: "<!DOCTYPE html><html></html>",
310+
},
311+
{
312+
description: "allows /_next/image requests with local URLs",
313+
url: "https://example.com/_next/image?url=/local/image.jpg",
314+
headers: {} as HeadersInit,
315+
userWorkerResponse: {
316+
body: "local image data",
317+
headers: { "content-type": "image/jpeg" },
318+
status: 200,
319+
},
320+
expectedStatus: 200,
321+
expectedBody: "local image data",
322+
},
323+
{
324+
description: "allows /_next/image requests with 304 status",
325+
url: "https://example.com/_next/image?url=https://example.com/image.jpg",
326+
headers: {} as HeadersInit,
327+
userWorkerResponse: {
328+
body: null,
329+
headers: { "content-type": "text/html" },
330+
status: 304,
331+
},
332+
expectedStatus: 304,
333+
expectedBody: null,
334+
},
335+
])(
336+
"$description",
337+
async ({
338+
url,
339+
headers,
340+
userWorkerResponse,
341+
expectedStatus,
342+
expectedBody,
343+
}) => {
344+
const request = new Request(url, { headers });
345+
const ctx = createExecutionContext();
336346

337-
const env = {
338-
CONFIG: {
339-
has_user_worker: true,
340-
invoke_user_worker_ahead_of_assets: true,
341-
},
342-
USER_WORKER: {
343-
async fetch(_: Request): Promise<Response> {
344-
return new Response("local image data", {
345-
headers: { "content-type": "image/jpeg" },
346-
});
347+
const env = {
348+
CONFIG: {
349+
has_user_worker: true,
350+
invoke_user_worker_ahead_of_assets: true,
347351
},
348-
},
349-
} as Env;
350-
351-
const response = await worker.fetch(request, env, ctx);
352-
expect(response.status).toBe(200);
353-
expect(await response.text()).toBe("local image data");
354-
});
355-
356-
it("allows /_next/image requests with 304 status", async () => {
357-
const request = new Request(
358-
"https://example.com/_next/image?url=https://example.com/image.jpg"
359-
);
360-
const ctx = createExecutionContext();
361-
362-
const env = {
363-
CONFIG: {
364-
has_user_worker: true,
365-
invoke_user_worker_ahead_of_assets: true,
366-
},
367-
USER_WORKER: {
368-
async fetch(_: Request): Promise<Response> {
369-
return new Response(null, {
370-
status: 304,
371-
headers: { "content-type": "text/html" },
372-
});
352+
USER_WORKER: {
353+
async fetch(_: Request): Promise<Response> {
354+
return new Response(userWorkerResponse.body, {
355+
status: userWorkerResponse.status,
356+
headers: userWorkerResponse.headers,
357+
});
358+
},
373359
},
374-
},
375-
} as Env;
360+
} as Env;
376361

377-
const response = await worker.fetch(request, env, ctx);
378-
expect(response.status).toBe(304);
379-
});
362+
const response = await worker.fetch(request, env, ctx);
363+
expect(response.status).toBe(expectedStatus);
364+
if (expectedBody !== null) {
365+
expect(await response.text()).toBe(expectedBody);
366+
}
367+
}
368+
);
380369

381370
describe("free tier limiting", () => {
382371
it("returns fetch from asset worker for assets", async () => {

0 commit comments

Comments
 (0)