Attachment 171123 Details for Bug 36598 – Bug 36598: Prevent use of unsafe HTTP method with non-cud op parameter

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

Bug-36598-Prevent-use-of-unsafe-HTTP-method-with-n.patch (text/plain), 4.47 KB, created by Marcel de Rooy on 2024-09-06 07:40:30 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2024-09-06 07:40:30 UTC

Size: 4.47 KB

patch

obsolete

>From 1c57e640a508813d08c20779ef20e545302332f6 Mon Sep 17 00:00:00 2001
>From: Julian Maurice <julian.maurice@biblibre.com>
>Date: Tue, 14 May 2024 09:15:50 +0200
>Subject: [PATCH] Bug 36598: Prevent use of unsafe HTTP method with non-cud op
> parameter
>Content-Type: text/plain; charset=utf-8
>
>Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>---
> Koha/App/Plugin/CSRF.pm    | 11 ++++++++++-
> t/db_dependent/mojo/csrf.t | 26 ++++++++++++++++++++------
> 2 files changed, 30 insertions(+), 7 deletions(-)
>
>diff --git a/Koha/App/Plugin/CSRF.pm b/Koha/App/Plugin/CSRF.pm
>index ab84834919..01ecb3897d 100644
>--- a/Koha/App/Plugin/CSRF.pm
>+++ b/Koha/App/Plugin/CSRF.pm
>@@ -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);
>                 }
>diff --git a/t/db_dependent/mojo/csrf.t b/t/db_dependent/mojo/csrf.t
>index 4c67bfc2ef..70ee65a812 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 => 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' );
>+    };
> };
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 36598: 164847 | 164848 | 164870 | 165075 | 165076 | 166094 | 166634 | 166673 | 166675 | 166727 | 166728 | 166729 | 166730 | 166731 | 166732 | 166733 | 171119 | 171120 | 171121 | 171122 | 171123 | 171124 | 171125