Skip to content

Commit

Permalink
Merge pull request from GHSA-5p75-vc5g-8rv2
Browse files Browse the repository at this point in the history
* 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 <[email protected]>

* Update packages/kit/types/index.d.ts

* Update cool-lies-fly.md

* Update packages/kit/src/utils/http.js

Co-authored-by: Dominik G. <[email protected]>

* Update packages/kit/types/index.d.ts

Co-authored-by: Dominik G. <[email protected]>

* test(csrf): include additional methods and content-types

---------

Co-authored-by: Conduitry <[email protected]>
Co-authored-by: Dominik G. <[email protected]>
  • Loading branch information
3 people authored Apr 4, 2023
1 parent 0e4b9b1 commit bb2253d
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 15 deletions.
5 changes: 5 additions & 0 deletions .changeset/cool-lies-fly.md
Original file line number Diff line number Diff line change
@@ -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
9 changes: 6 additions & 3 deletions packages/kit/src/runtime/server/respond.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`);
Expand Down
9 changes: 8 additions & 1 deletion packages/kit/src/utils/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
);
}
28 changes: 20 additions & 8 deletions packages/kit/test/apps/basics/test/server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
});
});

Expand Down
6 changes: 3 additions & 3 deletions packages/kit/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit bb2253d

Please sign in to comment.