From bb2253d51d00aba2e4353952d4fb0dcde6c77123 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Tue, 4 Apr 2023 10:42:00 -0700 Subject: [PATCH] Merge pull request from GHSA-5p75-vc5g-8rv2 * fix: address security advisory CVE-2023-29003 by enabling CSRF protection for plain/text requests * Protect PUT/PATCH/DELETE. Add comment explicitly mentioning CSRF * update docs * update changeset * Update packages/kit/types/index.d.ts Co-authored-by: Conduitry * Update packages/kit/types/index.d.ts * Update cool-lies-fly.md * Update packages/kit/src/utils/http.js Co-authored-by: Dominik G. * Update packages/kit/types/index.d.ts Co-authored-by: Dominik G. * test(csrf): include additional methods and content-types --------- Co-authored-by: Conduitry Co-authored-by: Dominik G. --- .changeset/cool-lies-fly.md | 5 ++++ packages/kit/src/runtime/server/respond.js | 9 ++++-- packages/kit/src/utils/http.js | 9 +++++- .../kit/test/apps/basics/test/server.test.js | 28 +++++++++++++------ packages/kit/types/index.d.ts | 6 ++-- 5 files changed, 42 insertions(+), 15 deletions(-) create mode 100644 .changeset/cool-lies-fly.md diff --git a/.changeset/cool-lies-fly.md b/.changeset/cool-lies-fly.md new file mode 100644 index 000000000000..028deaf1579e --- /dev/null +++ b/.changeset/cool-lies-fly.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: address security advisory CVE-2023-29003 by including `text/plain` and `PUT`/`PATCH`/`DELETE` requests in set of blocked cross-origin requests for CSRF protection diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index 13d89588adca..f48bffefe849 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -51,9 +51,12 @@ export async function respond(request, options, manifest, state) { if (options.csrf_check_origin) { const forbidden = - request.method === 'POST' && - request.headers.get('origin') !== url.origin && - is_form_content_type(request); + is_form_content_type(request) && + (request.method === 'POST' || + request.method === 'PUT' || + request.method === 'PATCH' || + request.method === 'DELETE') && + request.headers.get('origin') !== url.origin; if (forbidden) { const csrf_error = error(403, `Cross-site ${request.method} form submissions are forbidden`); diff --git a/packages/kit/src/utils/http.js b/packages/kit/src/utils/http.js index 31721d10cc4a..f5bdf2ccff7c 100644 --- a/packages/kit/src/utils/http.js +++ b/packages/kit/src/utils/http.js @@ -68,5 +68,12 @@ export function is_content_type(request, ...types) { * @param {Request} request */ export function is_form_content_type(request) { - return is_content_type(request, 'application/x-www-form-urlencoded', 'multipart/form-data'); + // These content types must be protected against CSRF + // https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement/enctype + return is_content_type( + request, + 'application/x-www-form-urlencoded', + 'multipart/form-data', + 'text/plain' + ); } diff --git a/packages/kit/test/apps/basics/test/server.test.js b/packages/kit/test/apps/basics/test/server.test.js index f466878de0ba..ce31f520f98c 100644 --- a/packages/kit/test/apps/basics/test/server.test.js +++ b/packages/kit/test/apps/basics/test/server.test.js @@ -58,15 +58,27 @@ test.describe('Cookies', () => { test.describe('CSRF', () => { test('Blocks requests with incorrect origin', async ({ baseURL }) => { - const res = await fetch(`${baseURL}/csrf`, { - method: 'POST', - headers: { - 'content-type': 'application/x-www-form-urlencoded' + const content_types = [ + 'application/x-www-form-urlencoded', + 'multipart/form-data', + 'text/plain' + ]; + const methods = ['POST', 'PUT', 'PATCH', 'DELETE']; + for (const method of methods) { + for (const content_type of content_types) { + const res = await fetch(`${baseURL}/csrf`, { + method, + headers: { + 'content-type': content_type + } + }); + const message = `request method: ${method}, content-type: ${content_type}`; + expect(res.status, message).toBe(403); + expect(await res.text(), message).toBe( + `Cross-site ${method} form submissions are forbidden` + ); } - }); - - expect(res.status).toBe(403); - expect(await res.text()).toBe('Cross-site POST form submissions are forbidden'); + } }); }); diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index 19d6c8424f38..e3af084e6261 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -328,13 +328,13 @@ export interface KitConfig { reportOnly?: CspDirectives; }; /** - * Protection against [cross-site request forgery](https://owasp.org/www-community/attacks/csrf) attacks. + * Protection against [cross-site request forgery (CSRF)](https://owasp.org/www-community/attacks/csrf) attacks. */ csrf?: { /** - * Whether to check the incoming `origin` header for `POST` form submissions and verify that it matches the server's origin. + * Whether to check the incoming `origin` header for `POST`, `PUT`, `PATCH`, or `DELETE` form submissions and verify that it matches the server's origin. * - * To allow people to make `POST` form submissions to your app from other origins, you will need to disable this option. Be careful! + * To allow people to make `POST`, `PUT`, `PATCH`, or `DELETE` requests with a `Content-Type` of `application/x-www-form-urlencoded`, `multipart/form-data`, or `text/plain` to your app from other origins, you will need to disable this option. Be careful! * @default true */ checkOrigin?: boolean;