From cd7123734e344cafa0deca6318012dd2f8a46bd4 Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 6 Jun 2024 01:34:23 +0000 Subject: [PATCH] Bug 37040: Prevent ErrorDocument subrequests from activating CSRF 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 Signed-off-by: Matt Blenkinsop Signed-off-by: Martin Renvoize (cherry picked from commit c1ac12417612799a0055046e10031943adb02e18) Signed-off-by: Lucas Gass --- Koha/Middleware/CSRF.pm | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Koha/Middleware/CSRF.pm b/Koha/Middleware/CSRF.pm index b34208ec6c..8850d0ef80 100644 --- a/Koha/Middleware/CSRF.pm +++ b/Koha/Middleware/CSRF.pm @@ -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; } -- 2.39.5