From 68877fc0f50aa3f1fb92d73bde159d476cec1a54 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: Lucas Gass <lucas@bywatersolutions.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.5