Bug 12873 - Reserve can be cancelled by any logged in user
Summary: Reserve can be cancelled by any logged in user
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P3 critical (vote)
Target Milestone: Oldversions
Assignee: Rafal Kopaczka
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 12876
  Show dependency treegraph
 
Reported: 2014-09-04 14:40 UTC by Rafal Kopaczka
Modified: 2018-12-16 19:34 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
[PATCH] Bug 12873 - Reserve can be cancelled by any logged in user It is possible to cancel reservations through simply running opac-modreserve.pl with existing reserve_id number. This may provide remove even all reservations from system. The only limitat (1.47 KB, patch)
2014-09-04 14:54 UTC, Rafal Kopaczka
Details | Diff | Splinter Review
Bug 12873 - Reserve can be cancelled by any logged in user (1.52 KB, patch)
2014-09-04 15:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PASSED QA] Bug 12873 - Reserve can be cancelled by any logged in user (1.60 KB, patch)
2014-09-04 16:24 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Rafal Kopaczka 2014-09-04 14:40:34 UTC
It is possible to cancel reservations through simply running opac-modreserve.pl with existing reserve_id number. This may provide remove even all reservations from system. 
The only limitation is that user have to be logged in. 
Simplest solution is to check whether reserve belongs to user or not. 
A patch will follow.
Comment 1 Rafal Kopaczka 2014-09-04 14:54:37 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2014-09-04 15:07:41 UTC
This one smells critical.
Comment 3 Jonathan Druart 2014-09-04 15:15:07 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2014-09-04 15:21:55 UTC
In doubt, I put this bug report in Koha security.
I am under the impression that this bug should not be public and fix asap for all branches.
Comment 5 Martin Renvoize 2014-09-04 16:24:57 UTC
Created attachment 31379 [details] [review]
[PASSED QA] Bug 12873 - Reserve can be cancelled by any logged in user

It is possible to cancel reservations through simply running opac-modreserve.pl with existing reserve_id number. This may provide remove even all reservations from system. The only limitation is that user have to be logged in. Simplest solution is to check whether reserve belongs to user or not.

Test plan:
1. Create reserves by 2 different users, and get their ID's
2. Before patch, hold may by cancelled by anyone who run site:
http://example.com/cgi-bin/koha/opac-modrequest.pl?reserve_id=XXX
3. After patch hold may by cancelled only by user whose reserve is.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2014-09-04 16:26:39 UTC
Well caught.. makes me wonder if there are any similar issues lurking around.
Comment 7 Rafal Kopaczka 2014-09-05 08:58:00 UTC
Ouch, there's one more thing I forgot. Reserve may still be cancelled by owner even if it shouldn't be able to, when hold is in waiting status. Not so critical as previous but still. Add this as new bug or attach to this patch? That will be additional conditions only: 
($res->{wait} and not $res->{found}) or (not $res->{wait} and not $res->{inrtansit}) #taken from opac-user.pl
Comment 8 Jonathan Druart 2014-09-05 09:09:48 UTC
(In reply to Rafal Kopaczka from comment #7)
> Ouch, there's one more thing I forgot. Reserve may still be cancelled by
> owner even if it shouldn't be able to, when hold is in waiting status. Not
> so critical as previous but still. Add this as new bug or attach to this
> patch?

Yes, please open a new bug report.
Comment 9 Rafal Kopaczka 2014-09-05 10:56:16 UTC
(In reply to Martin Renvoize from comment #6)
> Well caught.. makes me wonder if there are any similar issues lurking around.

Same situation is in opac-modrequest-suspend.pl with ToggleSuspend function.
Comment 10 Rafal Kopaczka 2014-09-05 13:05:59 UTC
In bug 12876 is improved version of patch with more cases taken into account. Can someone take look at this? 
I've added function to check if reserve may be cancelled, this function should be also called in opac-user.pl, but there's big mess if it comes to reserve handling.
Comment 11 Tomás Cohen Arazi 2014-09-24 00:29:33 UTC
Patch pushed to master.

Thanks Rafal!
Comment 12 Galen Charlton 2014-10-01 21:48:43 UTC
Pushed to 3.16.x for inclusion in 3.16.4.
Comment 13 Fridolin SOMERS 2014-10-02 12:04:58 UTC
Pushed to 3.14.x, will be in 3.14.11