From f32c107197f99564c4c96e4abe0bd1b4bb795bea 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: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 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.2