From b5cae12aef10b667d1c8a29adc85a97addb6a753 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>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
---
 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