From ffad8932522163f31b1d5b1d9a0f8a68a28132af Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Wed, 10 May 2023 07:37:57 +0200
Subject: [PATCH] Bug 33702: Prevent ILL requests to be modified by somebody
 else

Same as previous patch, but for 'update' and 'cancreq'.
We remove the redirect, but here we only want to focus on the security
fix.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Confirmed. Without this patch a patron can modify and cancel any ILL
request in the OPAC. With this patch the patron is redirected to the
404 page if modification or cancellation is attempted.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
---
 opac/opac-illrequests.pl | 44 ++++++++++++++++------------------------
 1 file changed, 17 insertions(+), 27 deletions(-)

diff --git a/opac/opac-illrequests.pl b/opac/opac-illrequests.pl
index 0219174cb7..c97f32b47a 100755
--- a/opac/opac-illrequests.pl
+++ b/opac/opac-illrequests.pl
@@ -59,56 +59,46 @@ $template->param( backends_available => $backends_available );
 
 my $op = $params->{'method'} || 'list';
 
+my ( $illrequest_id, $request );
+if ( $illrequest_id = $params->{illrequest_id} ) {
+    $request = Koha::Illrequests->find($illrequest_id);
+    # Make sure the request belongs to the logged in user
+    unless ( $request->borrowernumber == $loggedinuser ) {
+        print $query->redirect("/cgi-bin/koha/errors/404.pl");
+        exit;
+    }
+}
+
 if ( $op eq 'list' ) {
 
     my $requests = Koha::Illrequests->search(
         { borrowernumber => $loggedinuser }
     );
-    my $req = Koha::Illrequest->new;
     $template->param(
         requests => $requests,
-        backends    => $backends
+        backends => $backends
     );
 
 } elsif ( $op eq 'view') {
-    my $request = Koha::Illrequests->find({
-        borrowernumber => $loggedinuser,
-        illrequest_id  => $params->{illrequest_id}
-    });
-    # Make sure the request belongs to the logged in user
-    unless ( $request->borrowernumber == $loggedinuser ) {
-        print $query->redirect("/cgi-bin/koha/errors/404.pl");
-        exit;
-    }
     $template->param(
         request => $request
     );
 
 } elsif ( $op eq 'update') {
-    my $request = Koha::Illrequests->find({
-        borrowernumber => $loggedinuser,
-        illrequest_id  => $params->{illrequest_id}
-    });
     $request->notesopac($params->{notesopac})->store;
     # Send a notice to staff alerting them of the update
     $request->send_staff_notice('ILL_REQUEST_MODIFIED');
     print $query->redirect(
-        '/cgi-bin/koha/opac-illrequests.pl?method=view&illrequest_id=' .
-        $params->{illrequest_id} .
-        '&message=1'
-    );
+            '/cgi-bin/koha/opac-illrequests.pl?method=view&illrequest_id='
+          . $illrequest_id
+          . '&message=1' );
     exit;
 } elsif ( $op eq 'cancreq') {
-    my $request = Koha::Illrequests->find({
-        borrowernumber => $loggedinuser,
-        illrequest_id  => $params->{illrequest_id}
-    });
     $request->status('CANCREQ')->store;
     print $query->redirect(
-        '/cgi-bin/koha/opac-illrequests.pl?method=view&illrequest_id=' .
-        $params->{illrequest_id} .
-        '&message=1'
-    );
+            '/cgi-bin/koha/opac-illrequests.pl?method=view&illrequest_id='
+          . $illrequest_id
+          . '&message=1' );
     exit;
 } elsif ( $op eq 'create' ) {
     if (!$params->{backend}) {
-- 
2.39.5