From 7819c0048d303b2aa12f334614f6aa2ae7fcff2f Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Mon, 15 Apr 2024 09:08:48 +0200 Subject: [PATCH] Bug 36598: Prohibit CUD operations with safe HTTP methods (GET/HEAD/...) Signed-off-by: Matt Blenkinsop Signed-off-by: Victor Grousset/tuxayo --- Koha/App/Plugin/CSRF.pm | 7 ++++++- t/db_dependent/mojo/csrf.t | 18 ++++++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/Koha/App/Plugin/CSRF.pm b/Koha/App/Plugin/CSRF.pm index 0d2846ae53..b24fef655a 100644 --- a/Koha/App/Plugin/CSRF.pm +++ b/Koha/App/Plugin/CSRF.pm @@ -62,7 +62,12 @@ sub register { my ( $next, $c, $action, $last ) = @_; my $method = $c->req->method; - if ( $method eq 'POST' || $method eq 'PUT' || $method eq 'DELETE' || $method eq 'PATCH' ) { + 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); + } + } else { if ( $c->cookie('CGISESSID') && !$self->is_csrf_valid( $c->req ) ) { return $c->reply->exception('Wrong CSRF token')->rendered(403); } diff --git a/t/db_dependent/mojo/csrf.t b/t/db_dependent/mojo/csrf.t index 02b707301d..d52dd77b8c 100755 --- a/t/db_dependent/mojo/csrf.t +++ b/t/db_dependent/mojo/csrf.t @@ -8,7 +8,7 @@ use Test::Mojo; use Koha::Database; subtest 'CSRF - Intranet' => sub { - plan tests => 4; + plan tests => 5; my $t = Test::Mojo->new('Koha::App::Intranet'); @@ -39,10 +39,17 @@ subtest 'CSRF - Intranet' => sub { $t->post_ok( '/cgi-bin/koha/mainpage.pl', form => { csrf_token => $csrf_token, op => 'cud-login' } ) ->status_is(200)->content_like( qr/Please log in again/, 'Login failed but CSRF test passed' ); }; + + subtest 'GETting what should be POSTed should fail' => 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"' ); + } }; subtest 'CSRF - OPAC' => sub { - plan tests => 4; + plan tests => 5; my $t = Test::Mojo->new('Koha::App::Opac'); @@ -73,4 +80,11 @@ subtest 'CSRF - OPAC' => sub { $t->post_ok( '/cgi-bin/koha/opac-user.pl', form => { csrf_token => $csrf_token, op => 'cud-login' } ) ->status_is(200)->content_like( qr/Log in to your account/, 'Login failed but CSRF test passed' ); }; + + subtest 'GETting what should be POSTed should fail' => 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"' ); + } }; -- 2.45.0