Skip to content

Commit 35eb321

Browse files
tusharpandey13Simen A. W. Olsen
andcommitted
fix: prevent OAuth parameter injection via returnTo (#2381)
- URL encode returnTo parameter to prevent injection of OAuth parameters - Add comprehensive unit tests for returnTo encoding scenarios - Tests cover basic encoding, OAuth param injection attempts, and edge cases Co-authored-by: Simen A. W. Olsen <my@simen.io>
1 parent 0055cc4 commit 35eb321

File tree

3 files changed

+90
-4
lines changed

3 files changed

+90
-4
lines changed

.gitignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,4 +141,5 @@ dist
141141
*.tmp
142142
*PLAN*.md
143143
.yalc/
144-
yalc.lock
144+
yalc.lock
145+
.npmrc

src/server/helpers/with-page-auth-required.test.ts

Lines changed: 87 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ describe("with-page-auth-required ssr", () => {
8181
);
8282
await expect(handler({})).rejects.toThrowError("NEXT_REDIRECT");
8383
expect(redirect).toHaveBeenCalledTimes(1);
84-
expect(redirect).toHaveBeenCalledWith("/auth/login?returnTo=/foo");
84+
expect(redirect).toHaveBeenCalledWith("/auth/login?returnTo=%2Ffoo");
8585
});
8686

8787
it("should protect a page and redirect to returnTo fn option", async () => {
@@ -114,7 +114,7 @@ describe("with-page-auth-required ssr", () => {
114114
).rejects.toThrowError("NEXT_REDIRECT");
115115
expect(redirect).toHaveBeenCalledTimes(1);
116116
expect(redirect).toHaveBeenCalledWith(
117-
"/auth/login?returnTo=/foo/bar?foo=bar"
117+
"/auth/login?returnTo=%2Ffoo%2Fbar%3Ffoo%3Dbar"
118118
);
119119
});
120120

@@ -165,6 +165,91 @@ describe("with-page-auth-required ssr", () => {
165165
expect(redirect).toHaveBeenCalledTimes(1);
166166
expect(redirect).toHaveBeenCalledWith("/api/auth/custom-login");
167167
});
168+
169+
it("should URL encode returnTo parameter to prevent OAuth param injection", async () => {
170+
const withPageAuthRequired = appRouteHandlerFactory(
171+
new Auth0Client({
172+
domain: constants.domain,
173+
clientId: constants.clientId,
174+
clientSecret: constants.clientSecret,
175+
appBaseUrl: constants.appBaseUrl,
176+
secret: constants.secret
177+
}),
178+
{
179+
loginUrl: "/auth/login"
180+
}
181+
);
182+
const handler = withPageAuthRequired(
183+
() => Promise.resolve(React.createElement("div", {}, "foo")),
184+
{
185+
returnTo:
186+
"/callback?scope=openid profile&audience=https://api.example.com"
187+
}
188+
);
189+
await expect(handler({})).rejects.toThrowError("NEXT_REDIRECT");
190+
expect(redirect).toHaveBeenCalledTimes(1);
191+
expect(redirect).toHaveBeenCalledWith(
192+
"/auth/login?returnTo=%2Fcallback%3Fscope%3Dopenid%20profile%26audience%3Dhttps%3A%2F%2Fapi.example.com"
193+
);
194+
});
195+
196+
it("should URL encode returnTo with special characters", async () => {
197+
const withPageAuthRequired = appRouteHandlerFactory(
198+
new Auth0Client({
199+
domain: constants.domain,
200+
clientId: constants.clientId,
201+
clientSecret: constants.clientSecret,
202+
appBaseUrl: constants.appBaseUrl,
203+
secret: constants.secret
204+
}),
205+
{
206+
loginUrl: "/auth/login"
207+
}
208+
);
209+
const handler = withPageAuthRequired(
210+
() => Promise.resolve(React.createElement("div", {}, "foo")),
211+
{
212+
returnTo: "/path/with spaces & special=chars"
213+
}
214+
);
215+
await expect(handler({})).rejects.toThrowError("NEXT_REDIRECT");
216+
expect(redirect).toHaveBeenCalledTimes(1);
217+
expect(redirect).toHaveBeenCalledWith(
218+
"/auth/login?returnTo=%2Fpath%2Fwith%20spaces%20%26%20special%3Dchars"
219+
);
220+
});
221+
222+
it("should URL encode returnTo from function to prevent OAuth param injection", async () => {
223+
const withPageAuthRequired = appRouteHandlerFactory(
224+
new Auth0Client({
225+
domain: constants.domain,
226+
clientId: constants.clientId,
227+
clientSecret: constants.clientSecret,
228+
appBaseUrl: constants.appBaseUrl,
229+
secret: constants.secret
230+
}),
231+
{
232+
loginUrl: "/auth/login"
233+
}
234+
);
235+
const handler = withPageAuthRequired(
236+
() => Promise.resolve(React.createElement("div", {}, "foo")),
237+
{
238+
async returnTo({ params }: any) {
239+
return `/callback/${(await params).id}?malicious=scope%3Dopenid`;
240+
}
241+
}
242+
);
243+
await expect(
244+
handler({
245+
params: Promise.resolve({ id: "123" })
246+
})
247+
).rejects.toThrowError("NEXT_REDIRECT");
248+
expect(redirect).toHaveBeenCalledTimes(1);
249+
expect(redirect).toHaveBeenCalledWith(
250+
"/auth/login?returnTo=%2Fcallback%2F123%3Fmalicious%3Dscope%253Dopenid"
251+
);
252+
});
168253
});
169254

170255
describe("pages router", () => {

src/server/helpers/with-page-auth-required.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ export const appRouteHandlerFactory =
196196
: opts.returnTo;
197197
const { redirect } = await import("next/navigation.js");
198198
redirect(
199-
`${config.loginUrl}${opts.returnTo ? `?returnTo=${returnTo}` : ""}`
199+
`${config.loginUrl}${returnTo ? `?returnTo=${encodeURIComponent(returnTo)}` : ""}`
200200
);
201201
}
202202
return handler(params);

0 commit comments

Comments
 (0)