Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 37040: Prevent ErrorDocument subrequests from activating CSRF

Browse source 
This change improves the mechanism for preventing the CSRF middleware
being activated by ErrorDocument subrequests.

This change was necessary due to a subtle issue identified by
Bug 37041.

Test plan:
0. Apply the patch
1. Restart Koha
koha-plack --restart kohadev
2. Go to http://localhost:8081/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=9908
3. Log in
4. Note that you get a pretty 403 and not an ugly plain text error
5. Go to http://localhost:8081
6. Fill in the login details, but use the HTML inspector to delete
the csrf_token from the hidden inputs
7. Submit the login
8. Note a pretty 403 page

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This commit is contained in:
David Cook 2024-06-06 01:34:23 +00:00 committed by Martin Renvoize
parent 1daccb71be
commit c1ac124176
Signed by: martin.renvoize
GPG key ID: 422B469130441A0F
1 changed files with 6 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
Koha/Middleware/CSRF.pm
View file

@ -43,6 +43,11 @@ sub call {
my $uri = $req->uri // q{};
my $referer = $req->referer // q{No referer};
#NOTE: Ignore ErrorDocument requests for CSRF
if ( $env->{'psgix.errordocument.SCRIPT_NAME'} ) {
return $self->app->($env);
}
my ($error);
if ( $stateless_methods{$request_method} && defined $original_op && $original_op =~ m{^cud-} ) {
$error = sprintf "Programming error - op '%s' must not start with 'cud-' for %s %s (referer: %s)", $original_op,
@ -73,14 +78,9 @@ sub call {
}
}
#NOTE: It is essential to check for this environmental variable.
#NOTE: If we don't check for it, then we'll also throw an error for the "subrequest" that ErrorDocument uses to
#fetch the error page document. Then we'll wind up with a very ugly error page and not our pretty one.
if ( $error && !$env->{'plack.middleware.Koha.CSRF'} ) {
if ( $error ) {
#NOTE: Other Middleware will take care of logging to correct place, as Koha::Logger doesn't know where to go here
warn $error;
$env->{'plack.middleware.Koha.CSRF'} = $error;
my $res = Plack::Response->new( 403, [ 'Content-Type' => 'text/plain' ], ["Wrong CSRF token"] );
return $res->finalize;
}