Bug 36598: Prevent use of unsafe HTTP method with non-cud op parameter

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Julian Maurice 2024-05-14 09:15:50 +02:00 committed by Lucas Gass
parent ec02f36f10
commit 0c6ef38fb3
2 changed files with 30 additions and 7 deletions

View File

@ -49,6 +49,10 @@ 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 the HTTP request method is unsafe (POST, PUT, DELETE, PATCH, CONNECT) and
the request contains an `op` parameter whose value does not start with "cud-",
it will immediately return a 400 response.
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.
@ -69,9 +73,14 @@ 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('Incorrect use of a safe HTTP method with a "cud-" op parameter')->rendered(400);
return $c->reply->exception('Incorrect use of a safe HTTP method with an `op` parameter that starts with "cud-"')->rendered(400);
}
} else {
my $op = $c->req->param('op');
if ( $op && $op !~ /^cud-/ ) {
return $c->reply->exception('Incorrect use of an unsafe HTTP method with an `op` parameter that does not start with "cud-"')->rendered(400);
}
if ( $c->cookie('CGISESSID') && !$self->is_csrf_valid( $c->req ) ) {
return $c->reply->exception('Wrong CSRF token')->rendered(403);
}

View File

@ -8,7 +8,7 @@ use Test::Mojo;
use Koha::Database;
subtest 'CSRF - Intranet' => sub {
plan tests => 5;
plan tests => 6;
my $t = Test::Mojo->new('Koha::App::Intranet');
@ -44,12 +44,19 @@ subtest 'CSRF - Intranet' => sub {
plan tests => 3;
$t->get_ok('/cgi-bin/koha/mainpage.pl?op=cud-login')->status_is(400)
->content_like( qr/Incorrect use of a safe HTTP method with a/, 'Body contains correct error message' );
}
->content_like( qr/Incorrect use of a safe HTTP method with an `op` parameter that starts with &quot;cud-&quot;/, 'Body contains correct error message' );
};
subtest 'POSTing what should be GET should fail' => sub {
plan tests => 3;
$t->post_ok('/cgi-bin/koha/mainpage.pl?op=login')->status_is(400)
->content_like( qr/Incorrect use of an unsafe HTTP method with an `op` parameter that does not start with &quot;cud-&quot;/, 'Body contains correct error message' );
};
};
subtest 'CSRF - OPAC' => sub {
plan tests => 5;
plan tests => 6;
my $t = Test::Mojo->new('Koha::App::Opac');
@ -85,6 +92,13 @@ 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/Incorrect use of a safe HTTP method with a/, 'Body contains correct error message' );
}
->content_like( qr/Incorrect use of a safe HTTP method with an `op` parameter that starts with &quot;cud-&quot;/, 'Body contains correct error message' );
};
subtest 'POSTing what should be GET should fail' => sub {
plan tests => 3;
$t->post_ok('/cgi-bin/koha/opac-user.pl?op=login')->status_is(400)
->content_like( qr/Incorrect use of an unsafe HTTP method with an `op` parameter that does not start with &quot;cud-&quot;/, 'Body contains correct error message' );
};
};