From ec02f36f10169a1400680487946ac572da671ad0 Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Mon, 13 May 2024 15:06:04 +0200 Subject: [PATCH] Bug 36598: Improve documentation and error message in CSRF plugin Signed-off-by: Victor Grousset/tuxayo Signed-off-by: Marcel de Rooy Signed-off-by: Lucas Gass --- Koha/App/Plugin/CSRF.pm | 18 +++++++++++------- t/db_dependent/mojo/csrf.t | 4 ++-- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/Koha/App/Plugin/CSRF.pm b/Koha/App/Plugin/CSRF.pm index 579d0178bd..ab84834919 100644 --- a/Koha/App/Plugin/CSRF.pm +++ b/Koha/App/Plugin/CSRF.pm @@ -43,14 +43,18 @@ use Koha::Token; Called by Mojolicious when the plugin is loaded. -Defines an `around_action` hook that will return a 403 response if CSRF token -is missing or invalid. +Defines an `around_action` hook that will prevent CSRF attacks. -This verification occurs only for HTTP methods POST, PUT, DELETE and PATCH. +If the HTTP request method is safe (GET, HEAD, OPTIONS, TRACE) and the request +contains an `op` parameter whose value starts with "cud-" (which means it's a +Create, Update or Delete operation), it will immediately return a 400 response. -If CGISESSID cookie is missing, it means that we are not authenticated or we -are authenticated to the API by another method (HTTP basic or OAuth2). In this -case, no verification is done. +If the HTTP request method is unsafe (POST, PUT, DELETE, PATCH, CONNECT) and +the CGISESSID cookie is set, the CSRF token is checked. A 403 response is +immediately returned if the CSRF token is missing or invalid. +If the CGISESSID cookie is missing, it means that we are not authenticated or +we are authenticated to the API by another method (HTTP basic or OAuth2). In +this case, no verification is done. =cut @@ -65,7 +69,7 @@ sub register { if ( $method eq 'GET' || $method eq 'HEAD' || $method eq 'OPTIONS' || $method eq 'TRACE' ) { my $op = $c->req->param('op'); if ( $op && $op =~ /^cud-/ ) { - return $c->reply->exception('Wrong HTTP method')->rendered(400); + return $c->reply->exception('Incorrect use of a safe HTTP method with a "cud-" op parameter')->rendered(400); } } else { if ( $c->cookie('CGISESSID') && !$self->is_csrf_valid( $c->req ) ) { diff --git a/t/db_dependent/mojo/csrf.t b/t/db_dependent/mojo/csrf.t index d52dd77b8c..4c67bfc2ef 100755 --- a/t/db_dependent/mojo/csrf.t +++ b/t/db_dependent/mojo/csrf.t @@ -44,7 +44,7 @@ subtest 'CSRF - Intranet' => sub { plan tests => 3; $t->get_ok('/cgi-bin/koha/mainpage.pl?op=cud-login')->status_is(400) - ->content_like( qr/Wrong HTTP method/, 'Body contains "Wrong HTTP method"' ); + ->content_like( qr/Incorrect use of a safe HTTP method with a/, 'Body contains correct error message' ); } }; @@ -85,6 +85,6 @@ subtest 'CSRF - OPAC' => sub { plan tests => 3; $t->get_ok('/cgi-bin/koha/opac-user.pl?op=cud-login')->status_is(400) - ->content_like( qr/Wrong HTTP method/, 'Body contains "Wrong HTTP method"' ); + ->content_like( qr/Incorrect use of a safe HTTP method with a/, 'Body contains correct error message' ); } }; -- 2.39.5