diff --git a/README.md b/README.md index 3e43a33ad..c73c2cb62 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,7 @@ The Auth0 Next.js SDK is a library for implementing user authentication in Next. - [API Reference](#api-reference) - [v1 Migration Guide](./V1_MIGRATION_GUIDE.md) - [Cookies and Security](#cookies-and-security) + - [Error Handling and Security](#error-handling-and-security) - [Base Path and Internationalized Routing](#base-path-and-internationalized-routing) - [Architecture](./ARCHITECTURE.md) - [Comparison with auth0-react](#comparison-with-auth0-react) @@ -188,6 +189,22 @@ The `HttpOnly` setting will make sure that client-side JavaScript is unable to a The `SameSite=Lax` setting will help mitigate CSRF attacks. Learn more about SameSite by reading the ["Upcoming Browser Behavior Changes: What Developers Need to Know"](https://auth0.com/blog/browser-behavior-changes-what-developers-need-to-know/) blog post. +### Error Handling and Security + +The default server side error handler for the `/api/auth/*` routes prints the error message to screen, eg + +```js +try { + await handler(req, res); +} catch (error) { + res.status(error.status || 400).end(error.message); +} +``` + +Because the error can come from the OpenID Connect `error` query parameter we do some [basic escaping](https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#rule-1-html-encode-before-inserting-untrusted-data-into-html-element-content) which makes sure the default error handler is safe from XSS. + +If you write your own error handler, you should **not** render the error message without using a templating engine that will properly escape it for other HTML contexts first. + ### Base Path and Internationalized Routing With Next.js you can deploy a Next.js application under a sub-path of a domain using [Base Path](https://nextjs.org/docs/api-reference/next.config.js/basepath) and serve internationalized (i18n) routes using [Internationalized Routing](https://nextjs.org/docs/advanced-features/i18n-routing). diff --git a/src/auth0-session/handlers/callback.ts b/src/auth0-session/handlers/callback.ts index 8328afbd0..b58d88279 100644 --- a/src/auth0-session/handlers/callback.ts +++ b/src/auth0-session/handlers/callback.ts @@ -11,6 +11,17 @@ function getRedirectUri(config: Config): string { return urlJoin(config.baseURL, config.routes.callback); } +// eslint-disable-next-line max-len +// Basic escaping for putting untrusted data directly into the HTML body, per: https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#rule-1-html-encode-before-inserting-untrusted-data-into-html-element-content +function htmlSafe(input: string): string { + return input + .replace(/&/g, '&') + .replace(//g, '>') + .replace(/"/g, '"') + .replace(/'/g, '''); +} + export type AfterCallback = (req: any, res: any, session: any, state: Record) => Promise | any; export type CallbackOptions = { @@ -47,7 +58,8 @@ export default function callbackHandlerFactory( state: expectedState }); } catch (err) { - throw new BadRequest(err.message); + // The error message can come from the route's query parameters, so do some basic escaping. + throw new BadRequest(htmlSafe(err.message)); } const openidState: { returnTo?: string } = decodeState(expectedState as string); diff --git a/tests/auth0-session/handlers/callback.test.ts b/tests/auth0-session/handlers/callback.test.ts index c0b5a4879..f12bca4c8 100644 --- a/tests/auth0-session/handlers/callback.test.ts +++ b/tests/auth0-session/handlers/callback.test.ts @@ -239,6 +239,14 @@ describe('callback', () => { expect(session.claims).toEqual(expect.objectContaining(expected)); }); + it('should escape html in error qp', async () => { + const baseURL = await setup(defaultConfig); + + await expect(get(baseURL, '/callback?error=')).rejects.toThrowError( + '<script>alert(1)</script>' + ); + }); + it("should expose all tokens when id_token is valid and response_type is 'code id_token'", async () => { const baseURL = await setup({ ...defaultConfig, @@ -377,7 +385,7 @@ describe('callback', () => { const redirectUri = 'http://messi:3000/api/auth/callback/runtime'; const baseURL = await setup(defaultConfig, { callbackOptions: { redirectUri } }); const state = encodeState({ foo: 'bar' }); - const cookieJar = toSignedCookieJar( { state, nonce: '__test_nonce__' }, baseURL); + const cookieJar = toSignedCookieJar({ state, nonce: '__test_nonce__' }, baseURL); const { res } = await post(baseURL, '/callback', { body: { state: state, diff --git a/tests/handlers/callback.test.ts b/tests/handlers/callback.test.ts index 95390dae8..28a7be2d8 100644 --- a/tests/handlers/callback.test.ts +++ b/tests/handlers/callback.test.ts @@ -89,6 +89,13 @@ describe('callback handler', () => { ).rejects.toThrow('unexpected iss value, expected https://acme.auth0.local/, got: other-issuer'); }); + it('should escape html in error qp', async () => { + const baseUrl = await setup(withoutApi); + await expect(get(baseUrl, `/api/auth/callback?error=`)).rejects.toThrow( + '<script>alert(1)</script>' + ); + }); + test('should create the session without OIDC claims', async () => { const baseUrl = await setup(withoutApi); const state = encodeState({ returnTo: baseUrl });