Skip to content

Commit b066cf8

Browse files
matthewdavidrodgersmatthewdavidrodgers
authored
Block requests vulnerable to opennext vulnerability (#9635)
Addresses GHSA-rvpw-p7vw-wj3m This has already been applied in production, this commit keeps the repository in sync. Images loaded via the /_next/image?url=<some remote image> were vulnerable to SSRF, and particularly if the user loaded the image in a new tab (instead of forcing the browser to treat it as an image). Thus, if request didn't indicate that the browser was going to treat the resource as an image (via the 'Sec-Fetch-Dest: image' header), we block the request when it doesn't return image content.
1 parent a3318f5 commit b066cf8

File tree

4 files changed

+193
-0
lines changed

4 files changed

+193
-0
lines changed

‎.changeset/eleven-fans-smoke.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+
Block possibly vulnerable requests to the router worker

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ type Data = {
3333
userWorkerAhead?: boolean;
3434
// double6 - Routing performed based on the _routes.json (if provided)
3535
staticRoutingDecision?: STATIC_ROUTING_DECISION;
36+
// double7 - Whether the request was blocked by abuse mitigation or not
37+
abuseMitigationBlocked?: boolean;
3638

3739
// -- Blobs --
3840
// blob1 - Hostname of the request
@@ -45,6 +47,8 @@ type Data = {
4547
version?: string;
4648
// blob5 - Region of the colo (e.g. WEUR)
4749
coloRegion?: string;
50+
// blob6 - URL for analysis
51+
abuseMitigationURLHost?: string;
4852
};
4953

5054
export class Analytics {
@@ -88,13 +92,15 @@ export class Analytics {
8892
? -1
8993
: Number(this.data.userWorkerAhead),
9094
this.data.staticRoutingDecision ?? STATIC_ROUTING_DECISION.NOT_PROVIDED, // double6
95+
this.data.abuseMitigationBlocked ? 1 : 0, // double7
9196
],
9297
blobs: [
9398
this.data.hostname?.substring(0, 256), // blob1 - trim to 256 bytes
9499
this.data.dispatchtype, // blob2
95100
this.data.error?.substring(0, 256), // blob3 - trim to 256 bytes
96101
this.data.version, // blob4
97102
this.data.coloRegion, // blob5
103+
this.data.abuseMitigationURLHost, // blob6
98104
],
99105
});
100106
}

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,24 @@ export default {
7777

7878
const maybeSecondRequest = request.clone();
7979

80+
let shouldBlockNonImageResponse = false;
81+
if (url.pathname === "/_next/image") {
82+
// is a next image
83+
const queryURLParam = url.searchParams.get("url");
84+
if (queryURLParam && !queryURLParam.startsWith("/")) {
85+
// that's a remote resource
86+
if (
87+
maybeSecondRequest.method !== "GET" ||
88+
maybeSecondRequest.headers.get("sec-fetch-dest") !== "image"
89+
) {
90+
// that was not loaded via a browser's <img> tag
91+
shouldBlockNonImageResponse = true;
92+
analytics.setData({ abuseMitigationURLHost: queryURLParam });
93+
}
94+
// otherwise, we're good
95+
}
96+
}
97+
8098
if (config.static_routing) {
8199
// evaluate "exclude" rules
82100
const excludeRulesMatcher = generateStaticRoutingRuleMatcher(
@@ -129,6 +147,17 @@ export default {
129147
});
130148

131149
userWorkerInvocation = true;
150+
if (shouldBlockNonImageResponse) {
151+
const resp = await env.USER_WORKER.fetch(maybeSecondRequest);
152+
if (
153+
!resp.headers.get("content-type")?.startsWith("image/") &&
154+
resp.status !== 304
155+
) {
156+
analytics.setData({ abuseMitigationBlocked: true });
157+
return new Response("Blocked", { status: 403 });
158+
}
159+
return resp;
160+
}
132161
return env.USER_WORKER.fetch(maybeSecondRequest);
133162
});
134163
}
@@ -158,6 +187,17 @@ export default {
158187
});
159188

160189
userWorkerInvocation = true;
190+
if (shouldBlockNonImageResponse) {
191+
const resp = await env.USER_WORKER.fetch(maybeSecondRequest);
192+
if (
193+
!resp.headers.get("content-type")?.startsWith("image/") &&
194+
resp.status !== 304
195+
) {
196+
analytics.setData({ abuseMitigationBlocked: true });
197+
return new Response("Blocked", { status: 403 });
198+
}
199+
return resp;
200+
}
161201
return env.USER_WORKER.fetch(maybeSecondRequest);
162202
});
163203
}
@@ -175,6 +215,17 @@ export default {
175215
});
176216

177217
userWorkerInvocation = true;
218+
if (shouldBlockNonImageResponse) {
219+
const resp = await env.USER_WORKER.fetch(maybeSecondRequest);
220+
if (
221+
!resp.headers.get("content-type")?.startsWith("image/") &&
222+
resp.status !== 304
223+
) {
224+
analytics.setData({ abuseMitigationBlocked: true });
225+
return new Response("Blocked", { status: 403 });
226+
}
227+
return resp;
228+
}
178229
return env.USER_WORKER.fetch(maybeSecondRequest);
179230
});
180231
}

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

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,4 +246,135 @@ describe("unit tests", async () => {
246246
const response = await worker.fetch(request, env, ctx);
247247
expect(await response.text()).toEqual("hello from user worker");
248248
});
249+
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+
);
254+
const ctx = createExecutionContext();
255+
256+
const env = {
257+
CONFIG: {
258+
has_user_worker: true,
259+
invoke_user_worker_ahead_of_assets: true,
260+
},
261+
USER_WORKER: {
262+
async fetch(_: Request): Promise<Response> {
263+
return new Response("<!DOCTYPE html><html></html>", {
264+
headers: { "content-type": "text/html" },
265+
});
266+
},
267+
},
268+
} as Env;
269+
270+
const response = await worker.fetch(request, env, ctx);
271+
expect(response.status).toBe(403);
272+
expect(await response.text()).toBe("Blocked");
273+
});
274+
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();
336+
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+
},
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+
});
373+
},
374+
},
375+
} as Env;
376+
377+
const response = await worker.fetch(request, env, ctx);
378+
expect(response.status).toBe(304);
379+
});
249380
});

0 commit comments

Comments
 (0)