From fa8256a40d63642121ea465899587ec6f3c98795 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Sch=C3=A4fer?= Date: Fri, 3 Jan 2025 16:10:15 +0100 Subject: [PATCH] Set a Path on the CSRF cookie Without setting a Path, the UA will infer the Path from the URL path, meaning that it will take the "directory name" of a URL path (so for a response for a resource at `/index`, the path would be inferred as `/`, and for a response for `/foo/bar` it would be inferred to be `/foo/` etc.). This is problematic with CSRF for multiple reasons: - It causes a proliferation of CSRF cookies which pollute client-side storage. - It breaks CSRF when requests are sent across paths. For example, if a resource at `/foo/bar` contains a form which submits to `/index`, they would be theoretically using different CSRF states. - Poem only processes a single cookie, which means that we have to rely on the ordering specified in RFC 6265 Section 5.4. This is bad for two reasons: 1. The ordering is only a SHOULD, not a MUST. 2. Poem does, in fact, not process cookies in the correct ordering (this is subject of another commit to fix). Note that this commit may break applications if they share a CSRF cookie name with other applications hosted at different paths on the same domain. --- poem/src/middleware/csrf.rs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/poem/src/middleware/csrf.rs b/poem/src/middleware/csrf.rs index 103bdeab62..aef7a42ffa 100644 --- a/poem/src/middleware/csrf.rs +++ b/poem/src/middleware/csrf.rs @@ -1,4 +1,4 @@ -use std::{sync::Arc, time::Duration}; +use std::{borrow::Cow, sync::Arc, time::Duration}; use base64::{engine::general_purpose::STANDARD, Engine}; use libcsrf::{ @@ -83,6 +83,10 @@ pub struct Csrf { http_only: bool, same_site: Option, ttl: Duration, + // Using Arc> here because the path is most likely going + // to be a static str, and if it's not, we don't want to have to copy it + // into every endpoint. + path: Arc>, } impl Default for Csrf { @@ -94,6 +98,7 @@ impl Default for Csrf { http_only: true, same_site: Some(SameSite::Strict), ttl: Duration::from_secs(24 * 60 * 60), + path: Arc::new(Cow::Borrowed("/")), } } } @@ -147,6 +152,17 @@ impl Csrf { } } + /// Set the path for which the CSRF cookie should be set. + /// + /// By default, this is `"/"`. + #[must_use] + pub fn path(self, value: impl Into>) -> Self { + Self { + path: Arc::new(value.into()), + ..self + } + } + /// Sets the protection ttl. This will be used for both the cookie /// expiry and the time window over which CSRF tokens are considered /// valid. @@ -170,6 +186,7 @@ impl Middleware for Csrf { http_only: self.http_only, same_site: self.same_site, ttl: self.ttl, + path: Arc::clone(&self.path), }) } } @@ -184,6 +201,7 @@ pub struct CsrfEndpoint { http_only: bool, same_site: Option, ttl: Duration, + path: Arc>, } impl CsrfEndpoint { @@ -226,6 +244,7 @@ impl Endpoint for CsrfEndpoint { cookie.set_http_only(self.http_only); cookie.set_same_site(self.same_site); cookie.set_max_age(self.ttl); + cookie.set_path(&**self.path); cookie };